Revert "Take all VPN underlying networks into account when migrating traffic for"
This reverts commit ee48ac4abd56e00836234be0556ef4437168bf4b.
Reason for revert: This change has been implicated in 4-way deadlocks as seen in b/134244752.
Bug: 134244752
Change-Id: Ibdaad3a4cbf0d8ef1ed53cfab1e454b9b878bae9
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 7e7fa1f..5027a12 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -4383,7 +4383,7 @@
/**
* @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) {
@@ -4395,24 +4395,17 @@
// see VpnService.setUnderlyingNetworks()'s javadoc about how to interpret
// the underlyingNetworks list.
if (underlyingNetworks == null) {
- NetworkAgentInfo defaultNai = getDefaultNetwork();
- if (defaultNai != null && defaultNai.linkProperties != null) {
- underlyingNetworks = new Network[] { defaultNai.network };
+ NetworkAgentInfo defaultNetwork = getDefaultNetwork();
+ if (defaultNetwork != null && defaultNetwork.linkProperties != null) {
+ 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) {
- 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;
+ return info.primaryUnderlyingIface == null ? null : info;
}
/**
diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java
index 9ed6cb9..b5b0384 100644
--- a/tests/net/java/android/net/NetworkStatsTest.java
+++ b/tests/net/java/android/net/NetworkStatsTest.java
@@ -569,7 +569,7 @@
.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 @@
.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.
diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
index c46534c..bce526d 100644
--- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -927,7 +927,7 @@
// 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 @@
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(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1650L, 150L, 0L, 0L, 1L)
- // VPN sent 1650 bytes over WiFi in foreground (SET_FOREGROUND).
- .addValues(TEST_IFACE, UID_VPN, SET_FOREGROUND, TAG_NONE, 0L, 0L, 1650L, 150L, 1L));
+ .addValues(
+ TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1650L, 150L, 1650L, 150L, 2L));
forcePollAndWaitForIdle();
@@ -964,7 +962,7 @@
// 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();
@@ -995,132 +993,6 @@
}
@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),
// but has declared only WiFi (TEST_IFACE) in its underlying network set.
@@ -1129,7 +1001,7 @@
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();
@@ -1510,11 +1382,11 @@
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;
}