From c6d3f3cf25859fb9b956b88a916cdf37c9e769c6 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 18 Feb 2022 09:05:10 +0900 Subject: [PATCH 1/2] Use MessageUtils instead of hardcoded strings in policyNameOf. This makes the code easier to maintain because we do not need to manually add string representations, and because it will throw at static initialization time if the clas contains duplicate POLICY_xxx values. The memory overhead is likely negligible. Bug: 216567577 Test: new coverage in FullScoreTest Change-Id: Iab23d414c8e28ff7f26060ad44fa996f277d361f --- .../server/connectivity/FullScore.java | 23 +++++++------------ .../server/connectivity/FullScoreTest.kt | 2 ++ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java index aebb80df38..fe1067eeb0 100644 --- a/service/src/com/android/server/connectivity/FullScore.java +++ b/service/src/com/android/server/connectivity/FullScore.java @@ -21,8 +21,6 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkScore.KEEP_CONNECTED_NONE; -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 android.annotation.IntDef; @@ -31,8 +29,10 @@ import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkScore; import android.net.NetworkScore.KeepConnectedReason; +import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.MessageUtils; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -112,21 +112,14 @@ public class FullScore { private static final long EXTERNAL_POLICIES_MASK = 0x00000000FFFFFFFFL & ~(1L << POLICY_YIELD_TO_BAD_WIFI); + private static SparseArray sMessageNames = MessageUtils.findMessageNames( + new Class[]{FullScore.class, NetworkScore.class}, new String[]{"POLICY_"}); + @VisibleForTesting static @NonNull String policyNameOf(final int policy) { - switch (policy) { - case POLICY_IS_VALIDATED: return "IS_VALIDATED"; - case POLICY_IS_VPN: return "IS_VPN"; - case POLICY_EVER_USER_SELECTED: return "EVER_USER_SELECTED"; - case POLICY_ACCEPT_UNVALIDATED: return "ACCEPT_UNVALIDATED"; - case POLICY_IS_UNMETERED: return "IS_UNMETERED"; - case POLICY_YIELD_TO_BAD_WIFI: return "YIELD_TO_BAD_WIFI"; - case POLICY_TRANSPORT_PRIMARY: return "TRANSPORT_PRIMARY"; - case POLICY_EXITING: return "EXITING"; - case POLICY_IS_INVINCIBLE: return "INVINCIBLE"; - case POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD: return "EVER_VALIDATED"; - } - throw new IllegalArgumentException("Unknown policy : " + policy); + final String name = sMessageNames.get(policy); + if (name == null) throw new IllegalArgumentException("Unknown policy: " + policy); + return name.substring("POLICY_".length()); } // Bitmask of all the policies applied to this score. diff --git a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt index 785153a0e4..3eaf63e0af 100644 --- a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt +++ b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt @@ -26,6 +26,7 @@ import androidx.test.filters.SmallTest import com.android.server.connectivity.FullScore.MAX_CS_MANAGED_POLICY import com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED import com.android.server.connectivity.FullScore.POLICY_EVER_USER_SELECTED +import com.android.server.connectivity.FullScore.POLICY_IS_UNMETERED import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED import com.android.server.connectivity.FullScore.POLICY_IS_VPN import com.android.testutils.DevSdkIgnoreRule @@ -101,6 +102,7 @@ class FullScoreTest { assertFailsWith { FullScore.policyNameOf(MAX_CS_MANAGED_POLICY + 1) } + assertEquals("IS_UNMETERED", FullScore.policyNameOf(POLICY_IS_UNMETERED)) } fun getAllPolicies() = Regex("POLICY_.*").let { nameRegex -> From 8c7a8860f9fab510931dd2cb2e60d08d8637e70d Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 18 Feb 2022 18:41:30 +0900 Subject: [PATCH 2/2] Add an IS_DESTROYED flag to FullScore. This is being added as the lowest priority score factor, just above the tie-breakers. It ensures that a network that has been destroyed will lose to another identical network that has not been destroyed, but will otherwise be scored identically. The flag is a CS-managed flag that is stored in NetworkAgentInfo. Currently it is always false, but it will be populated in future CLs. Bug: 216567577 Test: atest FrameworksNetTests Change-Id: Ib1cd342ab7dfc4df45715da19b743d711fe8d605 --- .../server/connectivity/FullScore.java | 26 +++++++++++++++---- .../server/connectivity/NetworkAgentInfo.java | 15 ++++++++--- .../server/connectivity/NetworkRanker.java | 10 +++++++ .../server/connectivity/FullScoreTest.kt | 7 +++-- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java index fe1067eeb0..799f46b524 100644 --- a/service/src/com/android/server/connectivity/FullScore.java +++ b/service/src/com/android/server/connectivity/FullScore.java @@ -98,9 +98,17 @@ public class FullScore { /** @hide */ public static final int POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD = 57; + // The network agent has communicated that this network no longer functions, and the underlying + // native network has been destroyed. The network will still be reported to clients as connected + // until a timeout expires, the agent disconnects, or the network no longer satisfies requests. + // This network should lose to an identical network that has not been destroyed, but should + // otherwise be scored exactly the same. + /** @hide */ + public static final int POLICY_IS_DESTROYED = 56; + // To help iterate when printing @VisibleForTesting - static final int MIN_CS_MANAGED_POLICY = POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD; + static final int MIN_CS_MANAGED_POLICY = POLICY_IS_DESTROYED; @VisibleForTesting static final int MAX_CS_MANAGED_POLICY = POLICY_IS_VALIDATED; @@ -142,6 +150,7 @@ public class FullScore { * @param config the NetworkAgentConfig of the network * @param everValidated whether this network has ever validated * @param yieldToBadWiFi whether this network yields to a previously validated wifi gone bad + * @param destroyed whether this network has been destroyed pending a replacement connecting * @return a FullScore that is appropriate to use for ranking. */ // TODO : this shouldn't manage bad wifi avoidance – instead this should be done by the @@ -149,7 +158,7 @@ public class FullScore { // connectivity for backward compatibility. public static FullScore fromNetworkScore(@NonNull final NetworkScore score, @NonNull final NetworkCapabilities caps, @NonNull final NetworkAgentConfig config, - final boolean everValidated, final boolean yieldToBadWiFi) { + final boolean everValidated, final boolean yieldToBadWiFi, final boolean destroyed) { return withPolicies(score.getLegacyInt(), score.getPolicies(), score.getKeepConnectedReason(), caps.hasCapability(NET_CAPABILITY_VALIDATED), @@ -159,6 +168,7 @@ public class FullScore { config.explicitlySelected, config.acceptUnvalidated, yieldToBadWiFi, + destroyed, false /* invincible */); // only prospective scores can be invincible } @@ -167,7 +177,7 @@ public class FullScore { * * NetworkOffers have score filters that are compared to the scores of actual networks * to see if they could possibly beat the current satisfier. Some things the agent can't - * know in advance ; a good example is the validation bit – some networks will validate, + * know in advance; a good example is the validation bit – some networks will validate, * others won't. For comparison purposes, assume the best, so all possibly beneficial * networks will be brought up. * @@ -190,12 +200,14 @@ public class FullScore { final boolean everUserSelected = false; // Don't assume the user will accept unvalidated connectivity. final boolean acceptUnvalidated = false; + // A network can only be destroyed once it has connected. + final boolean destroyed = false; // A prospective score is invincible if the legacy int in the filter is over the maximum // score. final boolean invincible = score.getLegacyInt() > NetworkRanker.LEGACY_INT_MAX; return withPolicies(score.getLegacyInt(), score.getPolicies(), KEEP_CONNECTED_NONE, mayValidate, vpn, unmetered, everValidated, everUserSelected, acceptUnvalidated, - yieldToBadWiFi, invincible); + yieldToBadWiFi, destroyed, invincible); } /** @@ -211,7 +223,8 @@ public class FullScore { public FullScore mixInScore(@NonNull final NetworkCapabilities caps, @NonNull final NetworkAgentConfig config, final boolean everValidated, - final boolean yieldToBadWifi) { + final boolean yieldToBadWifi, + final boolean destroyed) { return withPolicies(mLegacyInt, mPolicies, mKeepConnectedReason, caps.hasCapability(NET_CAPABILITY_VALIDATED), caps.hasTransport(TRANSPORT_VPN), @@ -220,6 +233,7 @@ public class FullScore { config.explicitlySelected, config.acceptUnvalidated, yieldToBadWifi, + destroyed, false /* invincible */); // only prospective scores can be invincible } @@ -236,6 +250,7 @@ public class FullScore { final boolean everUserSelected, final boolean acceptUnvalidated, final boolean yieldToBadWiFi, + final boolean destroyed, final boolean invincible) { return new FullScore(legacyInt, (externalPolicies & EXTERNAL_POLICIES_MASK) | (isValidated ? 1L << POLICY_IS_VALIDATED : 0) @@ -245,6 +260,7 @@ public class FullScore { | (everUserSelected ? 1L << POLICY_EVER_USER_SELECTED : 0) | (acceptUnvalidated ? 1L << POLICY_ACCEPT_UNVALIDATED : 0) | (yieldToBadWiFi ? 1L << POLICY_YIELD_TO_BAD_WIFI : 0) + | (destroyed ? 1L << POLICY_IS_DESTROYED : 0) | (invincible ? 1L << POLICY_IS_INVINCIBLE : 0), keepConnectedReason); } diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index e917b3f1d6..1bdf50a1aa 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -106,6 +106,12 @@ import java.util.TreeSet; // or tunnel) but does not disconnect from the AP/cell tower, or // d. a stand-alone device offering a WiFi AP without an uplink for configuration purposes. // 5. registered, created, connected, validated +// 6. registered, created, connected, (validated or unvalidated), destroyed +// This is an optional state where the underlying native network is destroyed but the network is +// still connected for scoring purposes, so can satisfy requests, including the default request. +// It is used when the transport layer wants to replace a network with another network (e.g., +// when Wi-Fi has roamed to a different BSSID that is part of a different L3 network) and does +// not want the device to switch to another network until the replacement connects and validates. // // The device's default network connection: // ---------------------------------------- @@ -184,6 +190,9 @@ public class NetworkAgentInfo implements Comparable, NetworkRa // shows up in API calls, is able to satisfy NetworkRequests and can become the default network. // This is a sticky bit; once set it is never cleared. public boolean everConnected; + // Whether this network has been destroyed and is being kept temporarily until it is replaced. + public boolean destroyed; + // Set to true if this Network successfully passed validation or if it did not satisfy the // default NetworkRequest in which case validation will not be attempted. // This is a sticky bit; once set it is never cleared even if future validation attempts fail. @@ -746,7 +755,7 @@ public class NetworkAgentInfo implements Comparable, NetworkRa final NetworkCapabilities oldNc = networkCapabilities; networkCapabilities = nc; mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig, everValidatedForYield(), - yieldToBadWiFi()); + yieldToBadWiFi(), destroyed); final NetworkMonitorManager nm = mNetworkMonitor; if (nm != null) { nm.notifyNetworkCapabilitiesChanged(nc); @@ -961,7 +970,7 @@ public class NetworkAgentInfo implements Comparable, NetworkRa */ public void setScore(final NetworkScore score) { mScore = FullScore.fromNetworkScore(score, networkCapabilities, networkAgentConfig, - everValidatedForYield(), yieldToBadWiFi()); + everValidatedForYield(), yieldToBadWiFi(), destroyed); } /** @@ -971,7 +980,7 @@ public class NetworkAgentInfo implements Comparable, NetworkRa */ public void updateScoreForNetworkAgentUpdate() { mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig, - everValidatedForYield(), yieldToBadWiFi()); + everValidatedForYield(), yieldToBadWiFi(), destroyed); } private boolean everValidatedForYield() { diff --git a/service/src/com/android/server/connectivity/NetworkRanker.java b/service/src/com/android/server/connectivity/NetworkRanker.java index 43da1d09cc..babc3531bd 100644 --- a/service/src/com/android/server/connectivity/NetworkRanker.java +++ b/service/src/com/android/server/connectivity/NetworkRanker.java @@ -28,6 +28,7 @@ import static com.android.net.module.util.CollectionUtils.filter; 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_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD; +import static com.android.server.connectivity.FullScore.POLICY_IS_DESTROYED; import static com.android.server.connectivity.FullScore.POLICY_IS_INVINCIBLE; import static com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED; import static com.android.server.connectivity.FullScore.POLICY_IS_VPN; @@ -263,6 +264,15 @@ public class NetworkRanker { } } + // If two networks are equivalent, and one has been destroyed pending replacement, keep the + // other one. This ensures that when the replacement connects, it's preferred. + partitionInto(candidates, nai -> !nai.getScore().hasPolicy(POLICY_IS_DESTROYED), + accepted, rejected); + if (accepted.size() == 1) return accepted.get(0); + if (accepted.size() > 0 && rejected.size() > 0) { + candidates = new ArrayList<>(accepted); + } + // 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; diff --git a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt index 3eaf63e0af..e7f6245547 100644 --- a/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt +++ b/tests/unit/java/com/android/server/connectivity/FullScoreTest.kt @@ -26,6 +26,7 @@ import androidx.test.filters.SmallTest import com.android.server.connectivity.FullScore.MAX_CS_MANAGED_POLICY import com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED import com.android.server.connectivity.FullScore.POLICY_EVER_USER_SELECTED +import com.android.server.connectivity.FullScore.POLICY_IS_DESTROYED import com.android.server.connectivity.FullScore.POLICY_IS_UNMETERED import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED import com.android.server.connectivity.FullScore.POLICY_IS_VPN @@ -48,7 +49,8 @@ class FullScoreTest { validated: Boolean = false, vpn: Boolean = false, onceChosen: Boolean = false, - acceptUnvalidated: Boolean = false + acceptUnvalidated: Boolean = false, + destroyed: Boolean = false ): FullScore { val nac = NetworkAgentConfig.Builder().apply { setUnvalidatedConnectivityAcceptable(acceptUnvalidated) @@ -58,7 +60,7 @@ class FullScoreTest { if (vpn) addTransportType(NetworkCapabilities.TRANSPORT_VPN) if (validated) addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) }.build() - return mixInScore(nc, nac, validated, false /* yieldToBadWifi */) + return mixInScore(nc, nac, validated, false /* yieldToBadWifi */, destroyed) } @Test @@ -120,6 +122,7 @@ class FullScoreTest { assertTrue(ns.withPolicies(vpn = true).hasPolicy(POLICY_IS_VPN)) assertTrue(ns.withPolicies(onceChosen = true).hasPolicy(POLICY_EVER_USER_SELECTED)) assertTrue(ns.withPolicies(acceptUnvalidated = true).hasPolicy(POLICY_ACCEPT_UNVALIDATED)) + assertTrue(ns.withPolicies(destroyed = true).hasPolicy(POLICY_IS_DESTROYED)) } @Test