Revert "Take all VPN underlying networks into account when migrating traffic for"

This reverts commit 612520f544.

Reason for revert: This change has been implicated in 4-way deadlocks as seen in b/134244752.

Bug: 134244752
Change-Id: I2f1839d7776a613ca571af8a542755ddc5fc8760
Merged-In: Ibdaad3a4cbf0d8ef1ed53cfab1e454b9b878bae9

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

@@ -4374,7 +4374,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
     /**
      * @return VPN information for accounting, or null if we can't retrieve all required
-     *         information, e.g underlying ifaces.
+     *         information, e.g primary underlying iface.
      */
     @Nullable
     private VpnInfo createVpnInfo(Vpn vpn) {
@@ -4386,24 +4386,17 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // see VpnService.setUnderlyingNetworks()'s javadoc about how to interpret
         // the underlyingNetworks list.
         if (underlyingNetworks == null) {
-            NetworkAgentInfo defaultNai = getDefaultNetwork();
+            NetworkAgentInfo defaultNetwork = getDefaultNetwork();
-            if (defaultNai != null && defaultNai.linkProperties != null) {
+            if (defaultNetwork != null && defaultNetwork.linkProperties != null) {
-                underlyingNetworks = new Network[] { defaultNai.network };
+                info.primaryUnderlyingIface = getDefaultNetwork().linkProperties.getInterfaceName();
+            }
+        } else if (underlyingNetworks.length > 0) {
+            LinkProperties linkProperties = getLinkProperties(underlyingNetworks[0]);
+            if (linkProperties != null) {
+                info.primaryUnderlyingIface = linkProperties.getInterfaceName();
             }
         }
-        if (underlyingNetworks != null && underlyingNetworks.length > 0) {
+        return info.primaryUnderlyingIface == null ? null : info;
-            List<String> interfaces = new ArrayList<>();
-            for (Network network : underlyingNetworks) {
-                LinkProperties lp = getLinkProperties(network);
-                if (lp != null) {
-                    interfaces.add(lp.getInterfaceName());
-                }
-            }
-            if (!interfaces.isEmpty()) {
-                info.underlyingIfaces = interfaces.toArray(new String[interfaces.size()]);
-            }
-        }
-        return info.underlyingIfaces == null ? null : info;
     }
 
     /**

tests/net/java/android/net/NetworkStatsTest.java

@@ -569,7 +569,7 @@ public class NetworkStatsTest {
             .addValues(underlyingIface, tunUid, SET_FOREGROUND, TAG_NONE, METERED_NO, ROAMING_NO,
                     DEFAULT_NETWORK_NO, 0L, 0L, 0L, 0L, 0L);
 
-        delta.migrateTun(tunUid, tunIface, new String[] {underlyingIface});
+        assertTrue(delta.toString(), delta.migrateTun(tunUid, tunIface, underlyingIface));
         assertEquals(20, delta.size());
 
         // tunIface and TEST_IFACE entries are not changed.
@@ -650,7 +650,7 @@ public class NetworkStatsTest {
             .addValues(underlyingIface, tunUid, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
                     DEFAULT_NETWORK_NO,  75500L, 37L, 130000L, 70L, 0L);
 
-        delta.migrateTun(tunUid, tunIface, new String[]{underlyingIface});
+        assertTrue(delta.migrateTun(tunUid, tunIface, underlyingIface));
         assertEquals(9, delta.size());
 
         // tunIface entries should not be changed.

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

@@ -927,7 +927,7 @@ public class NetworkStatsServiceTest {
         // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE).
         expectDefaultSettings();
         NetworkState[] networkStates = new NetworkState[] {buildWifiState(), buildVpnState()};
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
+        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(TEST_IFACE)};
         expectNetworkStatsUidDetail(buildEmptyStats());
         expectBandwidthControlCheck();
 
@@ -947,10 +947,8 @@ public class NetworkStatsServiceTest {
         expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3)
                 .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 1L)
                 .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 500L, 50L, 500L, 50L, 1L)
-                // VPN received 1650 bytes over WiFi in background (SET_DEFAULT).
+                .addValues(
-                .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1650L, 150L, 0L, 0L, 1L)
+                    TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1650L, 150L, 1650L, 150L, 2L));
-                // VPN sent 1650 bytes over WiFi in foreground (SET_FOREGROUND).
-                .addValues(TEST_IFACE, UID_VPN, SET_FOREGROUND, TAG_NONE, 0L, 0L, 1650L, 150L, 1L));
 
         forcePollAndWaitForIdle();
 
@@ -964,7 +962,7 @@ public class NetworkStatsServiceTest {
         // WiFi network is connected and VPN is using WiFi (which has TEST_IFACE).
         expectDefaultSettings();
         NetworkState[] networkStates = new NetworkState[] {buildWifiState(), buildVpnState()};
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
+        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(TEST_IFACE)};
         expectNetworkStatsUidDetail(buildEmptyStats());
         expectBandwidthControlCheck();
 
@@ -994,132 +992,6 @@ public class NetworkStatsServiceTest {
         assertUidTotal(sTemplateWifi, UID_VPN, 0L, 0L, 0L, 0L, 0);
     }
 
-    @Test
-    public void vpnWithTwoUnderlyingIfaces_packetDuplication() throws Exception {
-        // WiFi and Cell networks are connected and VPN is using WiFi (which has TEST_IFACE) and
-        // Cell (which has TEST_IFACE2) and has declared both of them in its underlying network set.
-        // Additionally, VPN is duplicating traffic across both WiFi and Cell.
-        expectDefaultSettings();
-        NetworkState[] networkStates =
-                new NetworkState[] {
-                    buildWifiState(), buildMobile4gState(TEST_IFACE2), buildVpnState()
-                };
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE, TEST_IFACE2})};
-        expectNetworkStatsUidDetail(buildEmptyStats());
-        expectBandwidthControlCheck();
-
-        mService.forceUpdateIfaces(
-                new Network[] {WIFI_NETWORK, VPN_NETWORK},
-                vpnInfos,
-                networkStates,
-                getActiveIface(networkStates));
-        // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption
-        // overhead per packet):
-        // 1000 bytes (100 packets) were sent/received by UID_RED and UID_BLUE over VPN.
-        // VPN sent/received 4400 bytes (400 packets) over both WiFi and Cell (8800 bytes in total).
-        // Of 8800 bytes over WiFi/Cell, expect:
-        // - 500 bytes rx/tx each over WiFi/Cell attributed to both UID_RED and UID_BLUE.
-        // - 1200 bytes rx/tx each over WiFi/Cell for VPN_UID.
-        incrementCurrentTime(HOUR_IN_MILLIS);
-        expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 4)
-                .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 2L)
-                .addValues(TUN_IFACE, UID_BLUE, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 2L)
-                .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 2200L, 200L, 2200L, 200L, 2L)
-                .addValues(
-                    TEST_IFACE2, UID_VPN, SET_DEFAULT, TAG_NONE, 2200L, 200L, 2200L, 200L, 2L));
-
-        forcePollAndWaitForIdle();
-
-        assertUidTotal(sTemplateWifi, UID_RED, 500L, 50L, 500L, 50L, 1);
-        assertUidTotal(sTemplateWifi, UID_BLUE, 500L, 50L, 500L, 50L, 1);
-        assertUidTotal(sTemplateWifi, UID_VPN, 1200L, 100L, 1200L, 100L, 2);
-
-        assertUidTotal(buildTemplateMobileWildcard(), UID_RED, 500L, 50L, 500L, 50L, 1);
-        assertUidTotal(buildTemplateMobileWildcard(), UID_BLUE, 500L, 50L, 500L, 50L, 1);
-        assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 1200L, 100L, 1200L, 100L, 2);
-    }
-
-    @Test
-    public void vpnWithTwoUnderlyingIfaces_splitTraffic() throws Exception {
-        // WiFi and Cell networks are connected and VPN is using WiFi (which has TEST_IFACE) and
-        // Cell (which has TEST_IFACE2) and has declared both of them in its underlying network set.
-        // Additionally, VPN is arbitrarily splitting traffic across WiFi and Cell.
-        expectDefaultSettings();
-        NetworkState[] networkStates =
-                new NetworkState[] {
-                    buildWifiState(), buildMobile4gState(TEST_IFACE2), buildVpnState()
-                };
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE, TEST_IFACE2})};
-        expectNetworkStatsUidDetail(buildEmptyStats());
-        expectBandwidthControlCheck();
-
-        mService.forceUpdateIfaces(
-                new Network[] {WIFI_NETWORK, VPN_NETWORK},
-                vpnInfos,
-                networkStates,
-                getActiveIface(networkStates));
-        // create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption
-        // overhead per packet):
-        // 1000 bytes (100 packets) were sent/received by UID_RED over VPN.
-        // VPN sent/received 660 bytes (60 packets) over WiFi and 440 bytes (40 packets) over Cell.
-        // For UID_RED, expect 600 bytes attributed over WiFi and 400 bytes over Cell for both
-        // rx/tx.
-        // For UID_VPN, expect 60 bytes attributed over WiFi and 40 bytes over Cell for both rx/tx.
-        incrementCurrentTime(HOUR_IN_MILLIS);
-        expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 3)
-              .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 2L)
-              .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 660L, 60L, 660L, 60L, 1L)
-              .addValues(TEST_IFACE2, UID_VPN, SET_DEFAULT, TAG_NONE, 440L, 40L, 440L, 40L, 1L));
-
-        forcePollAndWaitForIdle();
-
-        assertUidTotal(sTemplateWifi, UID_RED, 600L, 60L, 600L, 60L, 1);
-        assertUidTotal(sTemplateWifi, UID_VPN, 60L, 0L, 60L, 0L, 1);
-
-        assertUidTotal(buildTemplateMobileWildcard(), UID_RED, 400L, 40L, 400L, 40L, 1);
-        assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 40L, 0L, 40L, 0L, 1);
-    }
-
-    @Test
-    public void vpnWithTwoUnderlyingIfaces_splitTrafficWithCompression() throws Exception {
-        // WiFi and Cell networks are connected and VPN is using WiFi (which has TEST_IFACE) and
-        // Cell (which has TEST_IFACE2) and has declared both of them in its underlying network set.
-        // Additionally, VPN is arbitrarily splitting compressed traffic across WiFi and Cell.
-        expectDefaultSettings();
-        NetworkState[] networkStates =
-                new NetworkState[] {
-                    buildWifiState(), buildMobile4gState(TEST_IFACE2), buildVpnState()
-                };
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE, TEST_IFACE2})};
-        expectNetworkStatsUidDetail(buildEmptyStats());
-        expectBandwidthControlCheck();
-
-        mService.forceUpdateIfaces(
-                new Network[] {WIFI_NETWORK, VPN_NETWORK},
-                vpnInfos,
-                networkStates,
-                getActiveIface(networkStates));
-        // create some traffic (assume 10 bytes of MTU for VPN interface:
-        // 1000 bytes (100 packets) were sent/received by UID_RED over VPN.
-        // VPN sent/received 600 bytes (60 packets) over WiFi and 200 bytes (20 packets) over Cell.
-        // For UID_RED, expect 600 bytes attributed over WiFi and 200 bytes over Cell for both
-        // rx/tx.
-        // UID_VPN gets nothing attributed to it (avoiding negative stats).
-        incrementCurrentTime(HOUR_IN_MILLIS);
-        expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 4)
-              .addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 1000L, 100L, 1L)
-              .addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 600L, 60L, 600L, 60L, 0L)
-              .addValues(TEST_IFACE2, UID_VPN, SET_DEFAULT, TAG_NONE, 200L, 20L, 200L, 20L, 0L));
-
-        forcePollAndWaitForIdle();
-
-        assertUidTotal(sTemplateWifi, UID_RED, 600L, 60L, 600L, 60L, 0);
-        assertUidTotal(sTemplateWifi, UID_VPN, 0L, 0L, 0L, 0L, 0);
-
-        assertUidTotal(buildTemplateMobileWildcard(), UID_RED, 200L, 20L, 200L, 20L, 0);
-        assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 0L, 0L, 0L, 0L, 0);
-    }
-
     @Test
     public void vpnWithIncorrectUnderlyingIface() throws Exception {
         // WiFi and Cell networks are connected and VPN is using Cell (which has TEST_IFACE2),
@@ -1129,7 +1001,7 @@ public class NetworkStatsServiceTest {
                 new NetworkState[] {
                     buildWifiState(), buildMobile4gState(TEST_IFACE2), buildVpnState()
                 };
-        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
+        VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(TEST_IFACE)};
         expectNetworkStatsUidDetail(buildEmptyStats());
         expectBandwidthControlCheck();
 
@@ -1505,11 +1377,11 @@ public class NetworkStatsServiceTest {
         return new NetworkState(info, prop, new NetworkCapabilities(), VPN_NETWORK, null, null);
     }
 
-    private static VpnInfo createVpnInfo(String[] underlyingIfaces) {
+    private static VpnInfo createVpnInfo(String underlyingIface) {
         VpnInfo info = new VpnInfo();
         info.ownerUid = UID_VPN;
         info.vpnIface = TUN_IFACE;
-        info.underlyingIfaces = underlyingIfaces;
+        info.primaryUnderlyingIface = underlyingIface;
         return info;
     }