From 66f497c90778fa4696ae6fbd786b310228aabde9 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 14 Feb 2020 17:05:02 +0900 Subject: [PATCH 1/2] [NS D03] Migrate the bad wifi avoidance policy Test: ConnectivityServiceTest Bug: 113554781 Change-Id: I688593cc0379a0d2042e30fbe83e549dcb02723e --- .../server/connectivity/NetworkRanker.java | 27 +++++++++++++++++-- .../server/connectivity/NetworkRankerTest.kt | 2 ++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkRanker.java b/services/core/java/com/android/server/connectivity/NetworkRanker.java index 1ae7dc5c36..c536ab25e9 100644 --- a/services/core/java/com/android/server/connectivity/NetworkRanker.java +++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java @@ -16,8 +16,13 @@ package com.android.server.connectivity; +import static android.net.NetworkScore.POLICY_IGNORE_ON_WIFI; + +import static com.android.internal.util.FunctionalUtils.findFirst; + import android.annotation.NonNull; import android.annotation.Nullable; +import android.net.NetworkCapabilities; import android.net.NetworkRequest; import java.util.ArrayList; @@ -37,15 +42,33 @@ public class NetworkRanker { @NonNull final Collection nais) { final ArrayList candidates = new ArrayList<>(nais); candidates.removeIf(nai -> !nai.satisfies(request)); + // Enforce policy. + filterBadWifiAvoidancePolicy(candidates); NetworkAgentInfo bestNetwork = null; int bestScore = Integer.MIN_VALUE; for (final NetworkAgentInfo nai : candidates) { - if (nai.getCurrentScore() > bestScore) { + final int score = nai.getCurrentScore(); + if (score > bestScore) { bestNetwork = nai; - bestScore = nai.getCurrentScore(); + bestScore = score; } } return bestNetwork; } + + // If some network with wifi transport is present, drop all networks with POLICY_IGNORE_ON_WIFI. + private void filterBadWifiAvoidancePolicy( + @NonNull final ArrayList candidates) { + final NetworkAgentInfo wifi = findFirst(candidates, + nai -> nai.networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) + && nai.everValidated + // Horrible hack : there is old UI that will let a user say they want to + // override the policy only for this network only at this time and it + // feeds into the following member. This old UI should probably be removed + // but for now keep backward compatibility. + && !nai.avoidUnvalidated); + if (null == wifi) return; // No wifi : this policy doesn't apply + candidates.removeIf(nai -> nai.getNetworkScore().hasPolicy(POLICY_IGNORE_ON_WIFI)); + } } diff --git a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt index 86c91165f6..a6b371a23b 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt +++ b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt @@ -16,6 +16,7 @@ package com.android.server.connectivity +import android.net.NetworkCapabilities import android.net.NetworkRequest import androidx.test.filters.SmallTest import androidx.test.runner.AndroidJUnit4 @@ -35,6 +36,7 @@ class NetworkRankerTest { 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 From 1355c8b25547cd54ebc543ab650606b40b381d83 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 17 Feb 2020 16:19:52 +0900 Subject: [PATCH 2/2] [NS D04] Implement a simple speed comparison between scores. This is not used yet to avoid making the change too big to review, but D05 will use this. Refer to D05 for how this is going to be used. Test: ConnectivityServiceTest Change-Id: I32c12702c8bce959682b02f157946d3f37b12a0c --- core/java/android/net/NetworkScore.java | 86 +++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/core/java/android/net/NetworkScore.java b/core/java/android/net/NetworkScore.java index d2e59eb893..d207e7960d 100644 --- a/core/java/android/net/NetworkScore.java +++ b/core/java/android/net/NetworkScore.java @@ -83,6 +83,65 @@ public final class NetworkScore implements Parcelable { uplinkBandwidthKBps = uplinkBandwidth; } + /** + * Evaluate whether a metrics codes for faster network is faster than another. + * + * This is a simple comparison of expected speeds. If either of the tested attributes + * are unknown, this returns zero. This implementation just assumes downlink bandwidth + * is more important than uplink bandwidth, which is more important than latency. This + * is not a very good way of evaluating network speed, but it's a start. + * TODO : do something more representative of how fast the network feels + * + * @param other the Metrics to evaluate against + * @return a negative integer, zero, or a positive integer as this metrics is worse than, + * equally good as (or unknown), or better than the passed Metrics. + * @see #compareToPreferringKnown(Metrics) + * @hide + */ + // Can't implement Comparable because this is @hide. + public int compareTo(@NonNull final Metrics other) { + if (downlinkBandwidthKBps != BANDWIDTH_UNKNOWN + && other.downlinkBandwidthKBps != BANDWIDTH_UNKNOWN) { + if (downlinkBandwidthKBps > other.downlinkBandwidthKBps) return 1; + if (downlinkBandwidthKBps < other.downlinkBandwidthKBps) return -1; + } + if (uplinkBandwidthKBps != BANDWIDTH_UNKNOWN + && other.uplinkBandwidthKBps != BANDWIDTH_UNKNOWN) { + if (uplinkBandwidthKBps > other.uplinkBandwidthKBps) return 1; + if (uplinkBandwidthKBps < other.uplinkBandwidthKBps) return -1; + } + if (latencyMs != LATENCY_UNKNOWN && other.latencyMs != LATENCY_UNKNOWN) { + // Latency : lower is better + if (latencyMs > other.latencyMs) return -1; + if (latencyMs < other.latencyMs) return 1; + } + return 0; + } + + /** + * Evaluate whether a metrics codes for faster network is faster than another. + * + * This is a simple comparison of expected speeds. If either of the tested attributes + * are unknown, this prefers the known attributes. This implementation just assumes + * downlink bandwidth is more important than uplink bandwidth, which is more important than + * latency. This is not a very good way of evaluating network speed, but it's a start. + * TODO : do something more representative of how fast the network feels + * + * @param other the Metrics to evaluate against + * @return a negative integer, zero, or a positive integer as this metrics is worse than, + * equally good as (or unknown), or better than the passed Metrics. + * @see #compareTo(Metrics) + * @hide + */ + public int compareToPreferringKnown(@NonNull final Metrics other) { + if (downlinkBandwidthKBps > other.downlinkBandwidthKBps) return 1; + if (downlinkBandwidthKBps < other.downlinkBandwidthKBps) return -1; + if (uplinkBandwidthKBps > other.uplinkBandwidthKBps) return 1; + if (uplinkBandwidthKBps < other.uplinkBandwidthKBps) return -1; + // Latency : lower is better + return -Integer.compare(latencyMs, other.latencyMs); + } + /** toString */ public String toString() { return "latency = " + latencyMs + " downlinkBandwidth = " + downlinkBandwidthKBps @@ -346,6 +405,33 @@ public final class NetworkScore implements Parcelable { return mLegacyScore; } + /** + * Use the metrics to evaluate whether a network is faster than another. + * + * This is purely based on the metrics, and explicitly ignores policy or exiting. It's + * provided to get a decision on two networks when policy can not decide, or to evaluate + * how a network is expected to compare to another if it should validate. + * + * @param other the score to evaluate against + * @return whether this network is probably faster than the other + * @hide + */ + public boolean probablyFasterThan(@NonNull final NetworkScore other) { + if (mLegacyScore > other.mLegacyScore) return true; + final int atEndToEnd = mEndToEndMetrics.compareTo(other.mEndToEndMetrics); + if (atEndToEnd > 0) return true; + if (atEndToEnd < 0) return false; + final int atLinkLayer = mLinkLayerMetrics.compareTo(other.mLinkLayerMetrics); + if (atLinkLayer > 0) return true; + if (atLinkLayer < 0) return false; + final int atEndToEndPreferringKnown = + mEndToEndMetrics.compareToPreferringKnown(other.mEndToEndMetrics); + if (atEndToEndPreferringKnown > 0) return true; + if (atEndToEndPreferringKnown < 0) return false; + // If this returns 0, neither is "probably faster" so just return false. + return mLinkLayerMetrics.compareToPreferringKnown(other.mLinkLayerMetrics) > 0; + } + /** Builder for NetworkScore. */ public static class Builder { private int mPolicy = 0;