Merge "Match underlying networks based on interface name"

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

@@ -1104,7 +1104,7 @@ public class VcnManagementService extends IVcnManagementService.Stub {
             final NetworkCapabilities result = ncBuilder.build();
             final VcnUnderlyingNetworkPolicy policy = new VcnUnderlyingNetworkPolicy(
                     mTrackingNetworkCallback
-                            .requiresRestartForImmutableCapabilityChanges(result),
+                            .requiresRestartForImmutableCapabilityChanges(result, linkProperties),
                     result);
 
             logVdbg("getUnderlyingNetworkPolicy() called for caps: " + networkCapabilities
@@ -1354,19 +1354,29 @@ public class VcnManagementService extends IVcnManagementService.Stub {
      * without requiring a Network restart.
      */
     private class TrackingNetworkCallback extends ConnectivityManager.NetworkCallback {
+        private final Object mLockObject = new Object();
         private final Map<Network, NetworkCapabilities> mCaps = new ArrayMap<>();
+        private final Map<Network, LinkProperties> mLinkProperties = new ArrayMap<>();
 
         @Override
         public void onCapabilitiesChanged(Network network, NetworkCapabilities caps) {
-            synchronized (mCaps) {
+            synchronized (mLockObject) {
                 mCaps.put(network, caps);
             }
         }
 
+        @Override
+        public void onLinkPropertiesChanged(Network network, LinkProperties lp) {
+            synchronized (mLockObject) {
+                mLinkProperties.put(network, lp);
+            }
+        }
+
         @Override
         public void onLost(Network network) {
-            synchronized (mCaps) {
+            synchronized (mLockObject) {
                 mCaps.remove(network);
+                mLinkProperties.remove(network);
             }
         }
 
@@ -1393,22 +1403,28 @@ public class VcnManagementService extends IVcnManagementService.Stub {
             return true;
         }
 
-        private boolean requiresRestartForImmutableCapabilityChanges(NetworkCapabilities caps) {
+        private boolean requiresRestartForImmutableCapabilityChanges(
+                NetworkCapabilities caps, LinkProperties lp) {
             if (caps.getSubscriptionIds() == null) {
                 return false;
             }
 
-            synchronized (mCaps) {
+            synchronized (mLockObject) {
-                for (NetworkCapabilities existing : mCaps.values()) {
+                // Search for an existing network (using interfce names)
-                    if (caps.getSubscriptionIds().equals(existing.getSubscriptionIds())
+                // TODO: Get network from NetworkFactory (if exists) for this match.
-                            && hasSameTransportsAndCapabilities(caps, existing)) {
+                for (Entry<Network, LinkProperties> lpEntry : mLinkProperties.entrySet()) {
-                        // Restart if any immutable capabilities have changed
+                    if (lp.getInterfaceName() != null
-                        return existing.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)
+                            && !lp.getInterfaceName().isEmpty()
+                            && Objects.equals(
+                                    lp.getInterfaceName(), lpEntry.getValue().getInterfaceName())) {
+                        return mCaps.get(lpEntry.getKey())
+                                        .hasCapability(NET_CAPABILITY_NOT_RESTRICTED)
                                 != caps.hasCapability(NET_CAPABILITY_NOT_RESTRICTED);
                     }
                 }
             }
 
+            // If no network found, by definition does not need restart.
             return false;
         }
 

tests/vcn/java/com/android/server/VcnManagementServiceTest.java

@@ -17,7 +17,9 @@
 package com.android.server;
 
 import static android.net.ConnectivityManager.NetworkCallback;
+import static android.net.NetworkCapabilities.NET_CAPABILITY_ENTERPRISE;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS;
+import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
@@ -67,7 +69,6 @@ import android.net.LinkProperties;
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.net.NetworkRequest;
-import android.net.TelephonyNetworkSpecifier;
 import android.net.Uri;
 import android.net.vcn.IVcnStatusCallback;
 import android.net.vcn.IVcnUnderlyingNetworkPolicyListener;
@@ -128,6 +129,15 @@ public class VcnManagementServiceTest {
     private static final VcnConfig TEST_VCN_CONFIG;
     private static final VcnConfig TEST_VCN_CONFIG_PKG_2;
     private static final int TEST_UID = Process.FIRST_APPLICATION_UID;
+    private static final String TEST_IFACE_NAME = "TEST_IFACE";
+    private static final String TEST_IFACE_NAME_2 = "TEST_IFACE2";
+    private static final LinkProperties TEST_LP_1 = new LinkProperties();
+    private static final LinkProperties TEST_LP_2 = new LinkProperties();
+
+    static {
+        TEST_LP_1.setInterfaceName(TEST_IFACE_NAME);
+        TEST_LP_2.setInterfaceName(TEST_IFACE_NAME_2);
+    }
 
     static {
         final Context mockConfigContext = mock(Context.class);
@@ -1034,8 +1044,7 @@ public class VcnManagementServiceTest {
         setupSubscriptionAndStartVcn(subId, subGrp, isVcnActive);
 
         return mVcnMgmtSvc.getUnderlyingNetworkPolicy(
-                getNetworkCapabilitiesBuilderForTransport(subId, transport).build(),
+                getNetworkCapabilitiesBuilderForTransport(subId, transport).build(), TEST_LP_1);
-                new LinkProperties());
     }
 
     private void checkGetRestrictedTransportsFromCarrierConfig(
@@ -1260,7 +1269,7 @@ public class VcnManagementServiceTest {
                 false /* expectRestricted */);
     }
 
-    private void setupTrackedCarrierWifiNetwork(NetworkCapabilities caps) {
+    private void setupTrackedNetwork(NetworkCapabilities caps, LinkProperties lp) {
         mVcnMgmtSvc.systemReady();
 
         final ArgumentCaptor<NetworkCallback> captor =
@@ -1269,7 +1278,10 @@ public class VcnManagementServiceTest {
                 .registerNetworkCallback(
                         eq(new NetworkRequest.Builder().clearCapabilities().build()),
                         captor.capture());
-        captor.getValue().onCapabilitiesChanged(mock(Network.class, CALLS_REAL_METHODS), caps);
+
+        Network mockNetwork = mock(Network.class, CALLS_REAL_METHODS);
+        captor.getValue().onCapabilitiesChanged(mockNetwork, caps);
+        captor.getValue().onLinkPropertiesChanged(mockNetwork, lp);
     }
 
     @Test
@@ -1279,7 +1291,7 @@ public class VcnManagementServiceTest {
                 getNetworkCapabilitiesBuilderForTransport(TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI)
                         .removeCapability(NET_CAPABILITY_NOT_RESTRICTED)
                         .build();
-        setupTrackedCarrierWifiNetwork(existingNetworkCaps);
+        setupTrackedNetwork(existingNetworkCaps, TEST_LP_1);
 
         // Trigger test without VCN instance alive; expect restart due to change of NOT_RESTRICTED
         // immutable capability
@@ -1288,7 +1300,7 @@ public class VcnManagementServiceTest {
                         getNetworkCapabilitiesBuilderForTransport(
                                         TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI)
                                 .build(),
-                        new LinkProperties());
+                        TEST_LP_1);
         assertTrue(policy.isTeardownRequested());
     }
 
@@ -1298,7 +1310,7 @@ public class VcnManagementServiceTest {
         final NetworkCapabilities existingNetworkCaps =
                 getNetworkCapabilitiesBuilderForTransport(TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI)
                         .build();
-        setupTrackedCarrierWifiNetwork(existingNetworkCaps);
+        setupTrackedNetwork(existingNetworkCaps, TEST_LP_1);
 
         final VcnUnderlyingNetworkPolicy policy =
                 startVcnAndGetPolicyForTransport(
@@ -1315,7 +1327,7 @@ public class VcnManagementServiceTest {
                         .addCapability(NET_CAPABILITY_NOT_RESTRICTED)
                         .removeCapability(NET_CAPABILITY_IMS)
                         .build();
-        setupTrackedCarrierWifiNetwork(existingNetworkCaps);
+        setupTrackedNetwork(existingNetworkCaps, TEST_LP_1);
 
         final VcnUnderlyingNetworkPolicy policy =
                 mVcnMgmtSvc.getUnderlyingNetworkPolicy(
@@ -1336,7 +1348,7 @@ public class VcnManagementServiceTest {
                 new NetworkCapabilities.Builder()
                         .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
                         .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
-                        .setNetworkSpecifier(new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID_2))
+                        .setSubscriptionIds(Collections.singleton(TEST_SUBSCRIPTION_ID_2))
                         .build();
 
         VcnUnderlyingNetworkPolicy policy =
@@ -1346,6 +1358,38 @@ public class VcnManagementServiceTest {
         assertEquals(nc, policy.getMergedNetworkCapabilities());
     }
 
+    /**
+     * Checks that networks with similar capabilities do not clobber each other.
+     *
+     * <p>In previous iterations, the VcnMgmtSvc used capability-matching to check if a network
+     * undergoing policy checks were the same as an existing networks. However, this meant that if
+     * there were newly added capabilities that the VCN did not check, two networks differing only
+     * by that capability would restart each other constantly.
+     */
+    @Test
+    public void testGetUnderlyingNetworkPolicySimilarNetworks() throws Exception {
+        NetworkCapabilities nc1 =
+                new NetworkCapabilities.Builder()
+                        .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
+                        .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
+                        .addCapability(NET_CAPABILITY_INTERNET)
+                        .setSubscriptionIds(Collections.singleton(TEST_SUBSCRIPTION_ID_2))
+                        .build();
+
+        NetworkCapabilities nc2 =
+                new NetworkCapabilities.Builder(nc1)
+                        .addCapability(NET_CAPABILITY_ENTERPRISE)
+                        .removeCapability(NET_CAPABILITY_NOT_RESTRICTED)
+                        .build();
+
+        setupTrackedNetwork(nc1, TEST_LP_1);
+
+        VcnUnderlyingNetworkPolicy policy = mVcnMgmtSvc.getUnderlyingNetworkPolicy(nc2, TEST_LP_2);
+
+        assertFalse(policy.isTeardownRequested());
+        assertEquals(nc2, policy.getMergedNetworkCapabilities());
+    }
+
     @Test(expected = SecurityException.class)
     public void testGetUnderlyingNetworkPolicyInvalidPermission() {
         doReturn(PackageManager.PERMISSION_DENIED)