From b887f60347390d71733966d2ff401382038dc243 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 29 Mar 2021 17:03:59 +0900 Subject: [PATCH] [NS09] Implement the new ranking code At this stage, this is turned off. Unit tests will be in a followup change. Test: In a followup Bug: 167544279 Change-Id: I4448a3546fbc1a3dddf757982c031c5f39ba2889 --- framework/src/android/net/NetworkScore.java | 2 +- .../android/server/ConnectivityService.java | 5 +- .../server/connectivity/FullScore.java | 4 +- .../server/connectivity/NetworkAgentInfo.java | 12 +- .../server/connectivity/NetworkOffer.java | 16 +- .../server/connectivity/NetworkRanker.java | 222 +++++++++++++++++- .../android/server/NetworkAgentWrapper.java | 25 +- .../server/ConnectivityServiceTest.java | 71 +++--- .../server/connectivity/NetworkRankerTest.kt | 12 +- 9 files changed, 308 insertions(+), 61 deletions(-) diff --git a/framework/src/android/net/NetworkScore.java b/framework/src/android/net/NetworkScore.java index 1c235f4701..0dee225a0e 100644 --- a/framework/src/android/net/NetworkScore.java +++ b/framework/src/android/net/NetworkScore.java @@ -170,7 +170,7 @@ public final class NetworkScore implements Parcelable { @Override public String toString() { - return "Score(" + mLegacyInt + ")"; + return "Score(" + mLegacyInt + " ; Policies : " + mPolicies + ")"; } @Override diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 6708fc380e..e17920383f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7742,7 +7742,7 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkAgentInfo bestNetwork = null; NetworkRequest bestRequest = null; for (final NetworkRequest req : nri.mRequests) { - bestNetwork = mNetworkRanker.getBestNetwork(req, nais); + bestNetwork = mNetworkRanker.getBestNetwork(req, nais, nri.getSatisfier()); // Stop evaluating as the highest possible priority request is satisfied. if (null != bestNetwork) { bestRequest = req; @@ -7995,7 +7995,6 @@ public class ConnectivityService extends IConnectivityManager.Stub @NonNull final NetworkOffer offer, @NonNull final NetworkRanker networkRanker) { final NetworkRequest activeRequest = nri.isBeingSatisfied() ? nri.getActiveRequest() : null; final NetworkAgentInfo satisfier = null != activeRequest ? nri.getSatisfier() : null; - final FullScore satisfierScore = null != satisfier ? satisfier.getScore() : null; // Multi-layer requests have a currently active request, the one being satisfied. // Since the system will try to bring up a better network than is currently satisfying @@ -8034,7 +8033,7 @@ public class ConnectivityService extends IConnectivityManager.Stub && satisfier.factorySerialNumber == offer.providerId; final boolean newNeeded = (currentlyServing || (activeRequest.canBeSatisfiedBy(offer.caps) - && networkRanker.mightBeat(activeRequest, satisfierScore, offer))); + && networkRanker.mightBeat(activeRequest, satisfier, offer))); if (newNeeded != oldNeeded) { if (newNeeded) { offer.onNetworkNeeded(activeRequest); diff --git a/services/core/java/com/android/server/connectivity/FullScore.java b/services/core/java/com/android/server/connectivity/FullScore.java index 117253f9c7..6e38e2d1ec 100644 --- a/services/core/java/com/android/server/connectivity/FullScore.java +++ b/services/core/java/com/android/server/connectivity/FullScore.java @@ -193,14 +193,14 @@ public class FullScore { // telephony factory, so that it depends on the carrier. For now this is handled by // connectivity for backward compatibility. public FullScore mixInScore(@NonNull final NetworkCapabilities caps, - @NonNull final NetworkAgentConfig config, final boolean avoidBadWiFi) { + @NonNull final NetworkAgentConfig config, final boolean yieldToBadWifi) { return withPolicies(mLegacyInt, mPolicies, mKeepConnectedReason, caps.hasCapability(NET_CAPABILITY_VALIDATED), caps.hasTransport(TRANSPORT_VPN), caps.hasCapability(NET_CAPABILITY_NOT_METERED), config.explicitlySelected, config.acceptUnvalidated, - avoidBadWiFi); + yieldToBadWifi); } // TODO : this shouldn't manage bad wifi avoidance – instead this should be done by the diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 3902573fa2..5d793fdda7 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -143,7 +143,7 @@ import java.util.TreeSet; // the network is no longer considered "lingering". After the linger timer expires, if the network // is satisfying one or more background NetworkRequests it is kept up in the background. If it is // not, ConnectivityService disconnects the NetworkAgent's AsyncChannel. -public class NetworkAgentInfo implements Comparable { +public class NetworkAgentInfo implements Comparable, NetworkRanker.Scoreable { @NonNull public NetworkInfo networkInfo; // This Network object should always be used if possible, so as to encourage reuse of the @@ -890,7 +890,15 @@ public class NetworkAgentInfo implements Comparable { return isVPN(); } - public FullScore getScore() { + // Caller must not mutate. This method is called frequently and making a defensive copy + // would be too expensive. This is used by NetworkRanker.Scoreable, so it can be compared + // against other scoreables. + @Override public NetworkCapabilities getCaps() { + return networkCapabilities; + } + + // NetworkRanker.Scoreable + @Override public FullScore getScore() { return mScore; } diff --git a/services/core/java/com/android/server/connectivity/NetworkOffer.java b/services/core/java/com/android/server/connectivity/NetworkOffer.java index a0d3924454..5336593b40 100644 --- a/services/core/java/com/android/server/connectivity/NetworkOffer.java +++ b/services/core/java/com/android/server/connectivity/NetworkOffer.java @@ -40,7 +40,7 @@ import java.util.Set; * * @hide */ -public class NetworkOffer { +public class NetworkOffer implements NetworkRanker.Scoreable { @NonNull public final FullScore score; @NonNull public final NetworkCapabilities caps; @NonNull public final INetworkOfferCallback callback; @@ -66,6 +66,20 @@ public class NetworkOffer { this.providerId = providerId; } + /** + * Get the score filter of this offer + */ + @Override @NonNull public FullScore getScore() { + return score; + } + + /** + * Get the capabilities filter of this offer + */ + @Override @NonNull public NetworkCapabilities getCaps() { + return caps; + } + /** * Tell the provider for this offer that the network is needed for a request. * @param request the request for which the offer is needed diff --git a/services/core/java/com/android/server/connectivity/NetworkRanker.java b/services/core/java/com/android/server/connectivity/NetworkRanker.java index 0e4dda2288..698744a57a 100644 --- a/services/core/java/com/android/server/connectivity/NetworkRanker.java +++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java @@ -16,31 +16,223 @@ package com.android.server.connectivity; +import static android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; +import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.NetworkScore.POLICY_EXITING; +import static android.net.NetworkScore.POLICY_TRANSPORT_PRIMARY; +import static android.net.NetworkScore.POLICY_YIELD_TO_BAD_WIFI; + +import static com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED; +import static com.android.server.connectivity.FullScore.POLICY_EVER_USER_SELECTED; +import static com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED; +import static com.android.server.connectivity.FullScore.POLICY_IS_VPN; + import android.annotation.NonNull; import android.annotation.Nullable; import android.net.NetworkCapabilities; import android.net.NetworkRequest; +import com.android.net.module.util.CollectionUtils; + +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.List; +import java.util.function.Predicate; /** * A class that knows how to find the best network matching a request out of a list of networks. */ public class NetworkRanker { + /** + * A class that can be scored against other scoreables. + */ + public interface Scoreable { + /** Get score of this scoreable */ + FullScore getScore(); + /** Get capabilities of this scoreable */ + NetworkCapabilities getCaps(); + } + + private static final boolean USE_POLICY_RANKING = false; + public NetworkRanker() { } + // TODO : move to module utils CollectionUtils. + @NonNull private static ArrayList filter(@NonNull final Collection source, + @NonNull final Predicate test) { + final ArrayList matches = new ArrayList<>(); + for (final T e : source) { + if (test.test(e)) { + matches.add(e); + } + } + return matches; + } + + /** * Find the best network satisfying this request among the list of passed networks. */ - // Almost equivalent to Collections.max(nais), but allows returning null if no network - // satisfies the request. @Nullable public NetworkAgentInfo getBestNetwork(@NonNull final NetworkRequest request, + @NonNull final Collection nais, + @Nullable final NetworkAgentInfo currentSatisfier) { + final ArrayList candidates = filter(nais, nai -> nai.satisfies(request)); + if (candidates.size() == 1) return candidates.get(0); // Only one potential satisfier + if (candidates.size() <= 0) return null; // No network can satisfy this request + if (USE_POLICY_RANKING) { + return getBestNetworkByPolicy(candidates, currentSatisfier); + } else { + return getBestNetworkByLegacyInt(candidates); + } + } + + // Transport preference order, if it comes down to that. + private static final int[] PREFERRED_TRANSPORTS_ORDER = { TRANSPORT_ETHERNET, TRANSPORT_WIFI, + TRANSPORT_BLUETOOTH, TRANSPORT_CELLULAR }; + + // Function used to partition a list into two working areas depending on whether they + // satisfy a predicate. All items satisfying the predicate will be put in |positive|, all + // items that don't will be put in |negative|. + // This is useful in this file because many of the ranking checks will retain only networks that + // satisfy a predicate if any of them do, but keep them all if all of them do. Having working + // areas is uncustomary in Java, but this function is called in a fairly intensive manner + // and doing allocation quite that often might affect performance quite badly. + private static void partitionInto(@NonNull final List source, @NonNull Predicate test, + @NonNull final List positive, @NonNull final List negative) { + positive.clear(); + negative.clear(); + for (final T item : source) { + if (test.test(item)) { + positive.add(item); + } else { + negative.add(item); + } + } + } + + @Nullable private T getBestNetworkByPolicy( + @NonNull List candidates, + @Nullable final T currentSatisfier) { + // Used as working areas. + final ArrayList accepted = + new ArrayList<>(candidates.size() /* initialCapacity */); + final ArrayList rejected = + new ArrayList<>(candidates.size() /* initialCapacity */); + + // The following tests will search for a network matching a given criterion. They all + // function the same way : if any network matches the criterion, drop from consideration + // all networks that don't. To achieve this, the tests below : + // 1. partition the list of remaining candidates into accepted and rejected networks. + // 2. if only one candidate remains, that's the winner : if accepted.size == 1 return [0] + // 3. if multiple remain, keep only the accepted networks and go on to the next criterion. + // Because the working areas will be wiped, a copy of the accepted networks needs to be + // made. + // 4. if none remain, the criterion did not help discriminate so keep them all. As an + // optimization, skip creating a new array and go on to the next criterion. + + // If there is a connected VPN, use it. + partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_IS_VPN), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + + // Selected & Accept-unvalidated policy : if any network has both of these, then don't + // choose one that doesn't. + partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_EVER_USER_SELECTED) + && nai.getScore().hasPolicy(POLICY_ACCEPT_UNVALIDATED), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + + // Yield to bad wifi policy : if any wifi has ever been validated, keep only networks + // that don't yield to such a wifi network. + final boolean anyWiFiEverValidated = CollectionUtils.any(candidates, + nai -> nai.getScore().hasPolicy(POLICY_EVER_USER_SELECTED) + && nai.getCaps().hasTransport(TRANSPORT_WIFI)); + if (anyWiFiEverValidated) { + partitionInto(candidates, nai -> !nai.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + } + + // If any network is validated (or should be accepted even if it's not validated), then + // don't choose one that isn't. + partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_IS_VALIDATED) + || nai.getScore().hasPolicy(POLICY_ACCEPT_UNVALIDATED), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + + // If any network is not exiting, don't choose one that is. + partitionInto(candidates, nai -> !nai.getScore().hasPolicy(POLICY_EXITING), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + + // TODO : If any network is unmetered, don't choose a metered network. + // This can't be implemented immediately because prospective networks are always + // considered unmetered because factories don't know if the network will be metered. + // Saying an unmetered network always beats a metered one would mean that when metered wifi + // is connected, the offer for telephony would beat WiFi but the actual metered network + // would lose, so we'd have an infinite loop where telephony would continually bring up + // a network that is immediately torn down. + // Fix this by getting the agent to tell connectivity whether the network they will + // bring up is metered. Cell knows that in advance, while WiFi has a good estimate and + // can revise it if the network later turns out to be metered. + // partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_IS_UNMETERED), + // accepted, rejected); + // if (accepted.size() == 1) return accepted.get(0); + // if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted); + + // If any network is for the default subscription, don't choose a network for another + // subscription with the same transport. + partitionInto(candidates, nai -> nai.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY), + accepted, rejected); + for (final Scoreable defaultSubNai : accepted) { + // Remove all networks without the DEFAULT_SUBSCRIPTION policy and the same transports + // as a network that has it. + final int[] transports = defaultSubNai.getCaps().getTransportTypes(); + candidates.removeIf(nai -> !nai.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY) + && Arrays.equals(transports, nai.getCaps().getTransportTypes())); + } + if (1 == candidates.size()) return candidates.get(0); + // It's guaranteed candidates.size() > 0 because there is at least one with DEFAULT_SUB + // policy and only those without it were removed. + + // If some of the networks have a better transport than others, keep only the ones with + // the best transports. + for (final int transport : PREFERRED_TRANSPORTS_ORDER) { + partitionInto(candidates, nai -> nai.getCaps().hasTransport(transport), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) { + candidates = new ArrayList<>(accepted); + break; + } + } + + // At this point there are still multiple networks passing all the tests above. If any + // of them is the previous satisfier, keep it. + if (candidates.contains(currentSatisfier)) return currentSatisfier; + + // If there are still multiple options at this point but none of them is any of the + // transports above, it doesn't matter which is returned. They are all the same. + return candidates.get(0); + } + + // TODO : switch to the policy implementation and remove + // Almost equivalent to Collections.max(nais), but allows returning null if no network + // satisfies the request. + private NetworkAgentInfo getBestNetworkByLegacyInt( @NonNull final Collection nais) { NetworkAgentInfo bestNetwork = null; int bestScore = Integer.MIN_VALUE; for (final NetworkAgentInfo nai : nais) { - if (!nai.satisfies(request)) continue; if (nai.getCurrentScore() > bestScore) { bestNetwork = nai; bestScore = nai.getCurrentScore(); @@ -63,19 +255,37 @@ public class NetworkRanker { * beat a current champion. * * @param request The request to evaluate against. - * @param championScore The currently best network for this request. + * @param champion The currently best network for this request. * @param offer The offer. * @return Whether the offer stands a chance to beat the champion. */ public boolean mightBeat(@NonNull final NetworkRequest request, - @Nullable final FullScore championScore, + @Nullable final NetworkAgentInfo champion, @NonNull final NetworkOffer offer) { // If this network can't even satisfy the request then it can't beat anything, not // even an absence of network. It can't satisfy it anyway. if (!request.canBeSatisfiedBy(offer.caps)) return false; // If there is no satisfying network, then this network can beat, because some network // is always better than no network. - if (null == championScore) return true; + if (null == champion) return true; + if (USE_POLICY_RANKING) { + // If there is no champion, the offer can always beat. + // Otherwise rank them. + final ArrayList candidates = new ArrayList<>(); + candidates.add(champion); + candidates.add(offer); + return offer == getBestNetworkByPolicy(candidates, champion); + } else { + return mightBeatByLegacyInt(request, champion.getScore(), offer); + } + } + + /** + * Returns whether an offer might beat a champion according to the legacy int. + */ + public boolean mightBeatByLegacyInt(@NonNull final NetworkRequest request, + @Nullable final FullScore championScore, + @NonNull final NetworkOffer offer) { final int offerIntScore; if (offer.caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) { // If the offer might have Internet access, then it might validate. diff --git a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java index 6245e8542b..40d068d7e3 100644 --- a/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java +++ b/tests/net/integration/util/com/android/server/NetworkAgentWrapper.java @@ -52,7 +52,6 @@ import android.util.Log; import android.util.Range; import com.android.net.module.util.ArrayTrackRecord; -import com.android.server.connectivity.ConnectivityConstants; import com.android.testutils.HandlerUtils; import com.android.testutils.TestableNetworkCallback; @@ -70,7 +69,7 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { private final ConditionVariable mDisconnected = new ConditionVariable(); private final ConditionVariable mPreventReconnectReceived = new ConditionVariable(); private final AtomicBoolean mConnected = new AtomicBoolean(false); - private int mScore; + private NetworkScore mScore; private NetworkAgent mNetworkAgent; private int mStartKeepaliveError = SocketKeepalive.ERROR_UNSUPPORTED; private int mStopKeepaliveError = SocketKeepalive.NO_KEEPALIVE; @@ -91,23 +90,23 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { mNetworkCapabilities.addTransportType(transport); switch (transport) { case TRANSPORT_ETHERNET: - mScore = 70; + mScore = new NetworkScore.Builder().setLegacyInt(70).build(); break; case TRANSPORT_WIFI: - mScore = 60; + mScore = new NetworkScore.Builder().setLegacyInt(60).build(); break; case TRANSPORT_CELLULAR: - mScore = 50; + mScore = new NetworkScore.Builder().setLegacyInt(50).build(); break; case TRANSPORT_WIFI_AWARE: - mScore = 20; + mScore = new NetworkScore.Builder().setLegacyInt(20).build(); break; case TRANSPORT_VPN: mNetworkCapabilities.removeCapability(NET_CAPABILITY_NOT_VPN); // VPNs deduce the SUSPENDED capability from their underlying networks and there // is no public API to let VPN services set it. mNetworkCapabilities.removeCapability(NET_CAPABILITY_NOT_SUSPENDED); - mScore = ConnectivityConstants.VPN_DEFAULT_SCORE; + mScore = new NetworkScore.Builder().setLegacyInt(101).build(); break; default: throw new UnsupportedOperationException("unimplemented network type"); @@ -201,16 +200,22 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork { } public void setScore(@NonNull final NetworkScore score) { - mScore = score.getLegacyInt(); + mScore = score; mNetworkAgent.sendNetworkScore(score); } public void adjustScore(int change) { - mScore += change; + final int newLegacyScore = mScore.getLegacyInt() + change; + final NetworkScore.Builder builder = new NetworkScore.Builder() + .setLegacyInt(newLegacyScore); + if (mNetworkCapabilities.hasTransport(TRANSPORT_WIFI) && newLegacyScore < 50) { + builder.setExiting(true); + } + mScore = builder.build(); mNetworkAgent.sendNetworkScore(mScore); } - public int getScore() { + public NetworkScore getScore() { return mScore; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index c3cb5f8092..a55366fe97 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -290,7 +290,6 @@ import com.android.internal.util.test.BroadcastInterceptingContext; import com.android.internal.util.test.FakeSettingsProvider; import com.android.net.module.util.ArrayTrackRecord; import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInfo; -import com.android.server.connectivity.ConnectivityConstants; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; import com.android.server.connectivity.NetworkAgentInfo; @@ -3033,8 +3032,9 @@ public class ConnectivityServiceTest { } NetworkCapabilities filter = new NetworkCapabilities(); + filter.addTransportType(TRANSPORT_CELLULAR); filter.addCapability(capability); - // Add NOT_VCN_MANAGED capability into filter unconditionally since some request will add + // Add NOT_VCN_MANAGED capability into filter unconditionally since some requests will add // NOT_VCN_MANAGED automatically but not for NetworkCapabilities, // see {@code NetworkCapabilities#deduceNotVcnManagedCapability} for more details. filter.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); @@ -3063,15 +3063,11 @@ public class ConnectivityServiceTest { // Now bring in a higher scored network. TestNetworkAgentWrapper testAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); - // Rather than create a validated network which complicates things by registering it's - // own NetworkRequest during startup, just bump up the score to cancel out the - // unvalidated penalty. - testAgent.adjustScore(40); - - // When testAgent connects, because of its 50 score (50 for cell + 40 adjustment score - // - 40 penalty for not being validated), it will beat the testFactory's offer, so - // the request will be removed. - testAgent.connect(false); + // When testAgent connects, because of its score (50 legacy int / cell transport) + // it will beat or equal the testFactory's offer, so the request will be removed. + // Note the agent as validated only if the capability is INTERNET, as it's the only case + // where it makes sense. + testAgent.connect(NET_CAPABILITY_INTERNET == capability /* validated */); testAgent.addCapability(capability); testFactory.expectRequestRemove(); testFactory.assertRequestCountEquals(0); @@ -3085,17 +3081,18 @@ public class ConnectivityServiceTest { testFactory.assertRequestCountEquals(0); assertFalse(testFactory.getMyStartRequested()); - // Make the test agent weak enough to have the exact same score as the - // factory (50 for cell + 40 adjustment -40 validation penalty - 5 adjustment). Make sure - // the factory doesn't see the request. + // If using legacy scores, make the test agent weak enough to have the exact same score as + // the factory (50 for cell - 5 adjustment). Make sure the factory doesn't see the request. + // If not using legacy score, this is a no-op and the "same score removes request" behavior + // has already been tested above. testAgent.adjustScore(-5); expectNoRequestChanged(testFactory); assertFalse(testFactory.getMyStartRequested()); - // Make the test agent weak enough to see the two requests (the one that was just sent, - // and either the default one or the one sent at the top of this test if the default - // won't be seen). - testAgent.adjustScore(-45); + // Make the test agent weak enough that the factory will see the two requests (the one that + // was just sent, and either the default one or the one sent at the top of this test if + // the default won't be seen). + testAgent.setScore(new NetworkScore.Builder().setLegacyInt(2).setExiting(true).build()); testFactory.expectRequestAdds(2); testFactory.assertRequestCountEquals(2); assertTrue(testFactory.getMyStartRequested()); @@ -3127,7 +3124,7 @@ public class ConnectivityServiceTest { assertTrue(testFactory.getMyStartRequested()); // Adjust the agent score up again. Expect the request to be withdrawn. - testAgent.adjustScore(50); + testAgent.setScore(new NetworkScore.Builder().setLegacyInt(50).build()); testFactory.expectRequestRemove(); testFactory.assertRequestCountEquals(0); assertFalse(testFactory.getMyStartRequested()); @@ -3165,25 +3162,39 @@ public class ConnectivityServiceTest { // Skipping VALIDATED and CAPTIVE_PORTAL as they're disallowed. } + @Ignore("Refactoring in progress b/184028345") @Test public void testRegisterIgnoringScore() throws Exception { mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); - mWiFiNetworkAgent.adjustScore(30); // score = 60 (wifi) + 30 = 90 + mWiFiNetworkAgent.setScore(new NetworkScore.Builder().setLegacyInt(90).build()); mWiFiNetworkAgent.connect(true /* validated */); // Make sure the factory sees the default network final NetworkCapabilities filter = new NetworkCapabilities(); + filter.addTransportType(TRANSPORT_CELLULAR); filter.addCapability(NET_CAPABILITY_INTERNET); filter.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); final HandlerThread handlerThread = new HandlerThread("testNetworkFactoryRequests"); handlerThread.start(); final MockNetworkFactory testFactory = new MockNetworkFactory(handlerThread.getLooper(), mServiceContext, "testFactory", filter, mCsHandlerThread); - testFactory.registerIgnoringScore(); - testFactory.expectRequestAdd(); + testFactory.register(); - mWiFiNetworkAgent.adjustScore(20); // exceed the maximum score - expectNoRequestChanged(testFactory); // still seeing the request + final MockNetworkFactory testFactoryAll = new MockNetworkFactory(handlerThread.getLooper(), + mServiceContext, "testFactoryAll", filter, mCsHandlerThread); + testFactoryAll.registerIgnoringScore(); + + // The regular test factory should not see the request, because WiFi is stronger than cell. + expectNoRequestChanged(testFactory); + // With ignoringScore though the request is seen. + testFactoryAll.expectRequestAdd(); + + // The legacy int will be ignored anyway, set the only other knob to true + mWiFiNetworkAgent.setScore(new NetworkScore.Builder().setLegacyInt(110) + .setTransportPrimary(true).build()); + + expectNoRequestChanged(testFactory); // still not seeing the request + expectNoRequestChanged(testFactoryAll); // still seeing the request mWiFiNetworkAgent.disconnect(); } @@ -6047,7 +6058,8 @@ public class ConnectivityServiceTest { // called again, it does. For example, connect Ethernet, but with a low score, such that it // does not become the default network. mEthernetNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET); - mEthernetNetworkAgent.adjustScore(-40); + mEthernetNetworkAgent.setScore( + new NetworkScore.Builder().setLegacyInt(30).setExiting(true).build()); mEthernetNetworkAgent.connect(false); waitForIdle(); verify(mStatsManager).notifyNetworkStatus(any(List.class), @@ -6922,8 +6934,6 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mMockVpn); callback.assertNoCallback(); - assertTrue(mMockVpn.getAgent().getScore() > mEthernetNetworkAgent.getScore()); - assertEquals(ConnectivityConstants.VPN_DEFAULT_SCORE, mMockVpn.getAgent().getScore()); assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); NetworkCapabilities nc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); @@ -11645,14 +11655,14 @@ public class ConnectivityServiceTest { new LinkProperties(), oemPaidNc); oemPaidAgent.connect(true); - // The oemPaidAgent has score 50 (default for cell) so it beats what the oemPaidFactory can + // The oemPaidAgent has score 50/cell transport, so it beats what the oemPaidFactory can // provide, therefore it loses the request. oemPaidFactory.expectRequestRemove(); oemPaidFactory.assertRequestCountEquals(0); expectNoRequestChanged(internetFactory); internetFactory.assertRequestCountEquals(0); - oemPaidAgent.adjustScore(-30); + oemPaidAgent.setScore(new NetworkScore.Builder().setLegacyInt(20).setExiting(true).build()); // Now the that the agent is weak, the oemPaidFactory can beat the existing network for the // OEM_PAID request. The internet factory however can't beat a network that has OEM_PAID // for the preference request, so it doesn't see the request. @@ -11682,7 +11692,8 @@ public class ConnectivityServiceTest { // Now WiFi connects and it's unmetered, but it's weaker than cell. mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); - mWiFiNetworkAgent.adjustScore(-30); // Not the best Internet network, but unmetered + mWiFiNetworkAgent.setScore(new NetworkScore.Builder().setLegacyInt(30).setExiting(true) + .build()); // Not the best Internet network, but unmetered mWiFiNetworkAgent.connect(true); // The OEM_PAID preference prefers an unmetered network to an OEM_PAID network, so diff --git a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt index 86c91165f6..1348c6a1ac 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt +++ b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt @@ -43,7 +43,7 @@ class NetworkRankerTest { val nais = scores.map { makeNai(true, it) } val bestNetwork = nais[2] // The one with the top score val someRequest = mock(NetworkRequest::class.java) - assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais)) + assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais, bestNetwork)) } @Test @@ -52,20 +52,20 @@ class NetworkRankerTest { makeNai(false, 60), makeNai(true, 23), makeNai(false, 68)) val bestNetwork = nais[1] // Top score that's satisfying val someRequest = mock(NetworkRequest::class.java) - assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais)) + assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais, nais[1])) } @Test fun testNoMatch() { val nais = listOf(makeNai(false, 20), makeNai(false, 50), makeNai(false, 90)) val someRequest = mock(NetworkRequest::class.java) - assertNull(ranker.getBestNetwork(someRequest, nais)) + assertNull(ranker.getBestNetwork(someRequest, nais, null)) } @Test fun testEmpty() { val someRequest = mock(NetworkRequest::class.java) - assertNull(ranker.getBestNetwork(someRequest, emptyList())) + assertNull(ranker.getBestNetwork(someRequest, emptyList(), null)) } // Make sure the ranker is "stable" (as in stable sort), that is, it always returns the FIRST @@ -75,10 +75,10 @@ class NetworkRankerTest { val nais1 = listOf(makeNai(true, 30), makeNai(true, 30), makeNai(true, 30), makeNai(true, 30), makeNai(true, 30), makeNai(true, 30)) val someRequest = mock(NetworkRequest::class.java) - assertEquals(nais1[0], ranker.getBestNetwork(someRequest, nais1)) + assertEquals(nais1[0], ranker.getBestNetwork(someRequest, nais1, nais1[0])) val nais2 = listOf(makeNai(true, 30), makeNai(true, 50), makeNai(true, 20), makeNai(true, 50), makeNai(true, 50), makeNai(true, 40)) - assertEquals(nais2[1], ranker.getBestNetwork(someRequest, nais2)) + assertEquals(nais2[1], ranker.getBestNetwork(someRequest, nais2, nais2[1])) } }