Merge "[NS03] Mix in other CS-managed properties" am: dc2fed3454

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1556101

Change-Id: I7b904af6c34190f53378cc1acc518840b527d434

framework/src/android/net/NetworkAgentConfig.java

@@ -50,7 +50,8 @@ public final class NetworkAgentConfig implements Parcelable {
      * ap in the wifi settings to trigger a connection is explicit.  A 3rd party app asking to
      * connect to a particular access point is also explicit, though this may change in the future
      * as we want apps to use the multinetwork apis.
-     *
+     * TODO : this is a bad name, because it sounds like the user just tapped on the network.
+     * It's not necessarily the case ; auto-reconnection to WiFi has this true for example.
      * @hide
      */
     public boolean explicitlySelected;

framework/src/android/net/NetworkScore.java

@@ -20,6 +20,8 @@ import android.annotation.NonNull;
 import android.os.Parcel;
 import android.os.Parcelable;
 
+import com.android.internal.annotations.VisibleForTesting;
+
 /**
  * Object representing the quality of a network as perceived by the user.
  *
@@ -35,6 +37,10 @@ public final class NetworkScore implements Parcelable {
 
     // Agent-managed policies
     // TODO : add them here, starting from 1
+    /** @hide */
+    public static final int MIN_AGENT_MANAGED_POLICY = 0;
+    /** @hide */
+    public static final int MAX_AGENT_MANAGED_POLICY = -1;
 
     // Bitmask of all the policies applied to this score.
     private final long mPolicies;
@@ -54,6 +60,14 @@ public final class NetworkScore implements Parcelable {
         return mLegacyInt;
     }
 
+    /**
+     * @return whether this score has a particular policy.
+     */
+    @VisibleForTesting
+    public boolean hasPolicy(final int policy) {
+        return 0 != (mPolicies & (1L << policy));
+    }
+
     @Override
     public String toString() {
         return "Score(" + mLegacyInt + ")";

services/core/java/com/android/server/ConnectivityService.java

@@ -2961,6 +2961,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 case NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED: {
                     if (nai.everConnected) {
                         loge("ERROR: cannot call explicitlySelected on already-connected network");
+                        // Note that if the NAI had been connected, this would affect the
+                        // score, and therefore would require re-mixing the score and performing
+                        // a rematch.
                     }
                     nai.networkAgentConfig.explicitlySelected = toBool(msg.arg1);
                     nai.networkAgentConfig.acceptUnvalidated = toBool(msg.arg1) && toBool(msg.arg2);
@@ -4045,6 +4048,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // network, we should respect the user's option and don't need to popup the
             // PARTIAL_CONNECTIVITY notification to user again.
             nai.networkAgentConfig.acceptPartialConnectivity = accept;
+            nai.updateScoreForNetworkAgentConfigUpdate();
             rematchAllNetworksAndRequests();
             sendUpdatedScoreToFactories(nai);
         }

services/core/java/com/android/server/connectivity/FullScore.java

@@ -16,9 +16,21 @@
 
 package com.android.server.connectivity;
 
+import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
+import static android.net.NetworkCapabilities.TRANSPORT_VPN;
+
+import android.annotation.IntDef;
 import android.annotation.NonNull;
+import android.net.NetworkAgentConfig;
+import android.net.NetworkCapabilities;
 import android.net.NetworkScore;
 
+import com.android.internal.annotations.VisibleForTesting;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.util.StringJoiner;
+
 /**
  * This class represents how desirable a network is.
  *
@@ -31,11 +43,54 @@ public class FullScore {
     // a migration.
     private final int mLegacyInt;
 
+    /** @hide */
+    @Retention(RetentionPolicy.SOURCE)
+    @IntDef(prefix = {"POLICY_"}, value = {
+            POLICY_IS_VALIDATED,
+            POLICY_IS_VPN,
+            POLICY_EVER_USER_SELECTED,
+            POLICY_ACCEPT_UNVALIDATED
+    })
+    public @interface Policy {
+    }
+
     // Agent-managed policies are in NetworkScore. They start from 1.
-    // CS-managed policies
+    // CS-managed policies, counting from 63 downward
     // This network is validated. CS-managed because the source of truth is in NetworkCapabilities.
+    /** @hide */
     public static final int POLICY_IS_VALIDATED = 63;
 
+    // This is a VPN and behaves as one for scoring purposes.
+    /** @hide */
+    public static final int POLICY_IS_VPN = 62;
+
+    // This network has been selected by the user manually from settings or a 3rd party app
+    // at least once. {@see NetworkAgentConfig#explicitlySelected}.
+    /** @hide */
+    public static final int POLICY_EVER_USER_SELECTED = 61;
+
+    // The user has indicated in UI that this network should be used even if it doesn't
+    // validate. {@see NetworkAgentConfig#acceptUnvalidated}.
+    /** @hide */
+    public static final int POLICY_ACCEPT_UNVALIDATED = 60;
+
+    // To help iterate when printing
+    @VisibleForTesting
+    static final int MIN_CS_MANAGED_POLICY = POLICY_ACCEPT_UNVALIDATED;
+    @VisibleForTesting
+    static final int MAX_CS_MANAGED_POLICY = POLICY_IS_VALIDATED;
+
+    @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";
+        }
+        throw new IllegalArgumentException("Unknown policy : " + policy);
+    }
+
     // Bitmask of all the policies applied to this score.
     private final long mPolicies;
 
@@ -45,12 +100,46 @@ public class FullScore {
     }
 
     /**
-     * Make a FullScore from a NetworkScore
+     * Given a score supplied by the NetworkAgent and CS-managed objects, produce a full score.
+     *
+     * @param score the score supplied by the agent
+     * @param caps the NetworkCapabilities of the network
+     * @param config the NetworkAgentConfig of the network
+     * @return an FullScore that is appropriate to use for ranking.
      */
-    public static FullScore withPolicy(@NonNull final NetworkScore originalScore,
+    public static FullScore fromNetworkScore(@NonNull final NetworkScore score,
-            final boolean isValidated) {
+            @NonNull final NetworkCapabilities caps, @NonNull final NetworkAgentConfig config) {
-        return new FullScore(originalScore.getLegacyInt(),
+        return withPolicies(score.getLegacyInt(), caps.hasCapability(NET_CAPABILITY_VALIDATED),
-                isValidated ? 1L << POLICY_IS_VALIDATED : 0L);
+                caps.hasTransport(TRANSPORT_VPN),
+                config.explicitlySelected,
+                config.acceptUnvalidated);
+    }
+
+    /**
+     * Return a new score given updated caps and config.
+     *
+     * @param caps the NetworkCapabilities of the network
+     * @param config the NetworkAgentConfig of the network
+     * @return a score with the policies from the arguments reset
+     */
+    public FullScore mixInScore(@NonNull final NetworkCapabilities caps,
+            @NonNull final NetworkAgentConfig config) {
+        return withPolicies(mLegacyInt, caps.hasCapability(NET_CAPABILITY_VALIDATED),
+                caps.hasTransport(TRANSPORT_VPN),
+                config.explicitlySelected,
+                config.acceptUnvalidated);
+    }
+
+    private static FullScore withPolicies(@NonNull final int legacyInt,
+            final boolean isValidated,
+            final boolean isVpn,
+            final boolean everUserSelected,
+            final boolean acceptUnvalidated) {
+        return new FullScore(legacyInt,
+                (isValidated         ? 1L << POLICY_IS_VALIDATED : 0)
+                | (isVpn             ? 1L << POLICY_IS_VPN : 0)
+                | (everUserSelected  ? 1L << POLICY_EVER_USER_SELECTED : 0)
+                | (acceptUnvalidated ? 1L << POLICY_ACCEPT_UNVALIDATED : 0));
     }
 
     /**
@@ -58,11 +147,65 @@ public class FullScore {
      * This will be removed before S is published.
      */
     public int getLegacyInt() {
-        return mLegacyInt;
+        return getLegacyInt(false /* pretendValidated */);
     }
 
+    public int getLegacyIntAsValidated() {
+        return getLegacyInt(true /* pretendValidated */);
+    }
+
+    // TODO : remove these two constants
+    // Penalty applied to scores of Networks that have not been validated.
+    private static final int UNVALIDATED_SCORE_PENALTY = 40;
+
+    // Score for a network that can be used unvalidated
+    private static final int ACCEPT_UNVALIDATED_NETWORK_SCORE = 100;
+
+    private int getLegacyInt(boolean pretendValidated) {
+        // If the user has chosen this network at least once, give it the maximum score when
+        // checking to pretend it's validated, or if it doesn't need to validate because the
+        // user said to use it even if it doesn't validate.
+        // This ensures that networks that have been selected in UI are not torn down before the
+        // user gets a chance to prefer it when a higher-scoring network (e.g., Ethernet) is
+        // available.
+        if (hasPolicy(POLICY_EVER_USER_SELECTED)
+                && (hasPolicy(POLICY_ACCEPT_UNVALIDATED) || pretendValidated)) {
+            return ACCEPT_UNVALIDATED_NETWORK_SCORE;
+        }
+
+        int score = mLegacyInt;
+        // Except for VPNs, networks are subject to a penalty for not being validated.
+        // Apply the penalty unless the network is a VPN, or it's validated or pretending to be.
+        if (!hasPolicy(POLICY_IS_VALIDATED) && !pretendValidated && !hasPolicy(POLICY_IS_VPN)) {
+            score -= UNVALIDATED_SCORE_PENALTY;
+        }
+        if (score < 0) score = 0;
+        return score;
+    }
+
+    /**
+     * @return whether this score has a particular policy.
+     */
+    @VisibleForTesting
+    public boolean hasPolicy(final int policy) {
+        return 0 != (mPolicies & (1L << policy));
+    }
+
+    // Example output :
+    // Score(50 ; Policies : EVER_USER_SELECTED&IS_VALIDATED)
     @Override
     public String toString() {
-        return "Score(" + mLegacyInt + ")";
+        final StringJoiner sj = new StringJoiner(
+                "&", // delimiter
+                "Score(" + mLegacyInt + " ; Policies : ", // prefix
+                ")"); // suffix
+        for (int i = NetworkScore.MIN_AGENT_MANAGED_POLICY;
+                i <= NetworkScore.MAX_AGENT_MANAGED_POLICY; ++i) {
+            if (hasPolicy(i)) sj.add(policyNameOf(i));
+        }
+        for (int i = MIN_CS_MANAGED_POLICY; i <= MAX_CS_MANAGED_POLICY; ++i) {
+            if (hasPolicy(i)) sj.add(policyNameOf(i));
+        }
+        return sj.toString();
     }
 }

services/core/java/com/android/server/connectivity/NetworkAgentInfo.java

@@ -17,7 +17,6 @@
 package com.android.server.connectivity;
 
 import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport;
-import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED;
 import static android.net.NetworkCapabilities.transportNamesOf;
 
 import android.annotation.NonNull;
@@ -358,12 +357,12 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         networkInfo = info;
         linkProperties = lp;
         networkCapabilities = nc;
-        mScore = mixInScore(score, nc);
+        networkAgentConfig = config;
+        setScore(score); // uses members networkCapabilities and networkAgentConfig
         clatd = new Nat464Xlat(this, netd, dnsResolver, deps);
         mConnService = connService;
         mContext = context;
         mHandler = handler;
-        networkAgentConfig = config;
         this.factorySerialNumber = factorySerialNumber;
         this.creatorUid = creatorUid;
         mQosCallbackTracker = qosCallbackTracker;
@@ -669,6 +668,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
             @NonNull final NetworkCapabilities nc) {
         final NetworkCapabilities oldNc = networkCapabilities;
         networkCapabilities = nc;
+        mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig);
         final NetworkMonitorManager nm = mNetworkMonitor;
         if (nm != null) {
             nm.notifyNetworkCapabilitiesChanged(nc);
@@ -846,30 +846,6 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         return isVPN();
     }
 
-    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. 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 = mScore.getLegacyInt();
-        if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
-            score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY;
-        }
-        if (score < 0) score = 0;
-        return score;
-    }
-
     // Return true on devices configured to ignore score penalty for wifi networks
     // that become unvalidated (b/31075769).
     private boolean ignoreWifiUnvalidationPenalty() {
@@ -882,22 +858,29 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     // 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);
+        return mScore.getLegacyInt();
     }
 
     // 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);
+        return mScore.getLegacyIntAsValidated();
     }
 
+    /**
+     * Mix-in the ConnectivityService-managed bits in the score.
+     */
     public void setScore(final NetworkScore score) {
-        mScore = mixInScore(score, networkCapabilities);
+        mScore = FullScore.fromNetworkScore(score, networkCapabilities, networkAgentConfig);
     }
 
-    private static FullScore mixInScore(@NonNull final NetworkScore score,
+    /**
-            @NonNull final NetworkCapabilities caps) {
+     * Update the ConnectivityService-managed bits in the score.
-        return FullScore.withPolicy(score, caps.hasCapability(NET_CAPABILITY_VALIDATED));
+     *
+     * Call this after updating the network agent config.
+     */
+    public void updateScoreForNetworkAgentConfigUpdate() {
+        mScore = mScore.mixInScore(networkCapabilities, networkAgentConfig);
     }
 
     /**

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -4328,6 +4328,7 @@ public class ConnectivityServiceTest {
         assertTrue(mPolicyTracker.shouldNotifyWifiUnvalidated());
     }
 
+    @Ignore("Refactoring in progress b/178071397")
     @Test
     public void testAvoidBadWifi() throws Exception {
         final ContentResolver cr = mServiceContext.getContentResolver();

tests/net/java/com/android/server/connectivity/FullScoreTest.kt

@@ -0,0 +1,134 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.connectivity
+
+import android.net.NetworkAgentConfig
+import android.net.NetworkCapabilities
+import android.text.TextUtils
+import android.util.ArraySet
+import androidx.test.filters.SmallTest
+import androidx.test.runner.AndroidJUnit4
+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_VALIDATED
+import com.android.server.connectivity.FullScore.POLICY_IS_VPN
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.collections.minOfOrNull
+import kotlin.collections.maxOfOrNull
+import kotlin.reflect.full.staticProperties
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+@RunWith(AndroidJUnit4::class)
+@SmallTest
+class FullScoreTest {
+    // Convenience methods
+    fun FullScore.withPolicies(
+        validated: Boolean = false,
+        vpn: Boolean = false,
+        onceChosen: Boolean = false,
+        acceptUnvalidated: Boolean = false
+    ): FullScore {
+        val nac = NetworkAgentConfig.Builder().apply {
+            setUnvalidatedConnectivityAcceptable(acceptUnvalidated)
+            setExplicitlySelected(onceChosen)
+        }.build()
+        val nc = NetworkCapabilities.Builder().apply {
+            if (vpn) addTransportType(NetworkCapabilities.TRANSPORT_VPN)
+            if (validated) addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
+        }.build()
+        return mixInScore(nc, nac)
+    }
+
+    @Test
+    fun testGetLegacyInt() {
+        val ns = FullScore(50, 0L /* policy */)
+        assertEquals(10, ns.legacyInt) // -40 penalty for not being validated
+        assertEquals(50, ns.legacyIntAsValidated)
+
+        val vpnNs = FullScore(101, 0L /* policy */).withPolicies(vpn = true)
+        assertEquals(101, vpnNs.legacyInt) // VPNs are not subject to unvalidation penalty
+        assertEquals(101, vpnNs.legacyIntAsValidated)
+        assertEquals(101, vpnNs.withPolicies(validated = true).legacyInt)
+        assertEquals(101, vpnNs.withPolicies(validated = true).legacyIntAsValidated)
+
+        val validatedNs = ns.withPolicies(validated = true)
+        assertEquals(50, validatedNs.legacyInt) // No penalty, this is validated
+        assertEquals(50, validatedNs.legacyIntAsValidated)
+
+        val chosenNs = ns.withPolicies(onceChosen = true)
+        assertEquals(10, chosenNs.legacyInt)
+        assertEquals(100, chosenNs.legacyIntAsValidated)
+        assertEquals(10, chosenNs.withPolicies(acceptUnvalidated = true).legacyInt)
+        assertEquals(50, chosenNs.withPolicies(acceptUnvalidated = true).legacyIntAsValidated)
+    }
+
+    @Test
+    fun testToString() {
+        val string = FullScore(10, 0L /* policy */)
+                .withPolicies(vpn = true, acceptUnvalidated = true).toString()
+        assertTrue(string.contains("Score(10"), string)
+        assertTrue(string.contains("ACCEPT_UNVALIDATED"), string)
+        assertTrue(string.contains("IS_VPN"), string)
+        assertFalse(string.contains("IS_VALIDATED"), string)
+        val foundNames = ArraySet<String>()
+        getAllPolicies().forEach {
+            val name = FullScore.policyNameOf(it.get() as Int)
+            assertFalse(TextUtils.isEmpty(name))
+            assertFalse(foundNames.contains(name))
+            foundNames.add(name)
+        }
+        assertFailsWith<IllegalArgumentException> {
+            FullScore.policyNameOf(MAX_CS_MANAGED_POLICY + 1)
+        }
+    }
+
+    fun getAllPolicies() = Regex("POLICY_.*").let { nameRegex ->
+        FullScore::class.staticProperties.filter { it.name.matches(nameRegex) }
+    }
+
+    @Test
+    fun testHasPolicy() {
+        val ns = FullScore(50, 0L /* policy */)
+        assertFalse(ns.hasPolicy(POLICY_IS_VALIDATED))
+        assertFalse(ns.hasPolicy(POLICY_IS_VPN))
+        assertFalse(ns.hasPolicy(POLICY_EVER_USER_SELECTED))
+        assertFalse(ns.hasPolicy(POLICY_ACCEPT_UNVALIDATED))
+        assertTrue(ns.withPolicies(validated = true).hasPolicy(POLICY_IS_VALIDATED))
+        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))
+    }
+
+    @Test
+    fun testMinMaxPolicyConstants() {
+        val policies = getAllPolicies()
+
+        policies.forEach { policy ->
+            assertTrue(policy.get() as Int >= FullScore.MIN_CS_MANAGED_POLICY)
+            assertTrue(policy.get() as Int <= FullScore.MAX_CS_MANAGED_POLICY)
+        }
+        assertEquals(FullScore.MIN_CS_MANAGED_POLICY,
+                policies.minOfOrNull { it.get() as Int })
+        assertEquals(FullScore.MAX_CS_MANAGED_POLICY,
+                policies.maxOfOrNull { it.get() as Int })
+    }
+}

tests/net/java/com/android/server/connectivity/LingerMonitorTest.java

@@ -357,7 +357,7 @@ public class LingerMonitorTest {
         caps.addTransportType(transport);
         NetworkAgentInfo nai = new NetworkAgentInfo(null, new Network(netId), info,
                 new LinkProperties(), caps, new NetworkScore.Builder().setLegacyInt(50).build(),
-                mCtx, null, new NetworkAgentConfig() /* config */, mConnService, mNetd,
+                mCtx, null, new NetworkAgentConfig.Builder().build(), mConnService, mNetd,
                 mDnsResolver, NetworkProvider.ID_NONE, Binder.getCallingUid(),
                 mQosCallbackTracker, new ConnectivityService.Dependencies());
         nai.everValidated = true;