Revert "NetworkStatsService: Fix getDetailedUidStats to take VPNs into account."

This reverts commit ed11b84131.

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

Bug: 134244752
Change-Id: I0c00e8f0e30cee987b71b561079a97bf09d4dae4
This commit is contained in:
Benedict Wong
2019-06-06 17:06:48 -07:00
committed by Lorenzo Colitti
parent 1fbf261c01
commit 055992b112
5 changed files with 16 additions and 266 deletions

View File

@@ -34,7 +34,6 @@ import libcore.util.EmptyArray;
import java.io.CharArrayWriter;
import java.io.PrintWriter;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
@@ -995,33 +994,23 @@ public class NetworkStats implements Parcelable {
if (limitUid == UID_ALL && limitTag == TAG_ALL && limitIfaces == INTERFACES_ALL) {
return;
}
filter(e -> (limitUid == UID_ALL || limitUid == e.uid)
&& (limitTag == TAG_ALL || limitTag == e.tag)
&& (limitIfaces == INTERFACES_ALL
|| ArrayUtils.contains(limitIfaces, e.iface)));
}
/**
* Only keep entries with {@link #set} value less than {@link #SET_DEBUG_START}.
*
* <p>This mutates the original structure in place.
*/
public void filterDebugEntries() {
filter(e -> e.set < SET_DEBUG_START);
}
private void filter(Predicate<Entry> predicate) {
Entry entry = new Entry();
int nextOutputEntry = 0;
for (int i = 0; i < size; i++) {
entry = getValues(i, entry);
if (predicate.test(entry)) {
if (nextOutputEntry != i) {
setValues(nextOutputEntry, entry);
}
final boolean matches =
(limitUid == UID_ALL || limitUid == entry.uid)
&& (limitTag == TAG_ALL || limitTag == entry.tag)
&& (limitIfaces == INTERFACES_ALL
|| ArrayUtils.contains(limitIfaces, entry.iface));
if (matches) {
setValues(nextOutputEntry, entry);
nextOutputEntry++;
}
}
size = nextOutputEntry;
}

View File

@@ -263,10 +263,6 @@ public class NetworkStatsFactory {
return stats;
}
/**
* @deprecated Use NetworkStatsService#getDetailedUidStats which also accounts for
* VPN traffic
*/
public NetworkStats readNetworkStatsDetail() throws IOException {
return readNetworkStatsDetail(UID_ALL, null, TAG_ALL, null);
}

View File

@@ -293,22 +293,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
/** Data layer operation counters for splicing into other structures. */
private NetworkStats mUidOperations = new NetworkStats(0L, 10);
/**
* Snapshot containing most recent network stats for all UIDs across all interfaces and tags
* since boot.
*
* <p>Maintains migrated VPN stats which are result of performing TUN migration on {@link
* #mLastUidDetailSnapshot}.
*/
@GuardedBy("mStatsLock")
private NetworkStats mTunAdjustedStats;
/**
* Used by {@link #mTunAdjustedStats} to migrate VPN traffic over delta between this snapshot
* and latest snapshot.
*/
@GuardedBy("mStatsLock")
private NetworkStats mLastUidDetailSnapshot;
/** Must be set in factory by calling #setHandler. */
private Handler mHandler;
private Handler.Callback mHandlerCallback;
@@ -828,39 +812,15 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override
public NetworkStats getDetailedUidStats(String[] requiredIfaces) {
try {
// Get the latest snapshot from NetworkStatsFactory.
// TODO: Querying for INTERFACES_ALL may incur performance penalty. Consider restricting
// this to limited set of ifaces.
NetworkStats uidDetailStats = getNetworkStatsUidDetail(INTERFACES_ALL);
// Migrate traffic from VPN UID over delta and update mTunAdjustedStats.
NetworkStats result;
synchronized (mStatsLock) {
migrateTunTraffic(uidDetailStats, mVpnInfos);
result = mTunAdjustedStats.clone();
}
// Apply filter based on ifacesToQuery.
final String[] ifacesToQuery =
NetworkStatsFactory.augmentWithStackedInterfaces(requiredIfaces);
result.filter(UID_ALL, ifacesToQuery, TAG_ALL);
return result;
return getNetworkStatsUidDetail(ifacesToQuery);
} catch (RemoteException e) {
Log.wtf(TAG, "Error compiling UID stats", e);
return new NetworkStats(0L, 0);
}
}
@VisibleForTesting
NetworkStats getTunAdjustedStats() {
synchronized (mStatsLock) {
if (mTunAdjustedStats == null) {
return null;
}
return mTunAdjustedStats.clone();
}
}
@Override
public String[] getMobileIfaces() {
return mMobileIfaces;
@@ -1335,34 +1295,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// a race condition between the service handler thread and the observer's
mStatsObservers.updateStats(xtSnapshot, uidSnapshot, new ArrayMap<>(mActiveIfaces),
new ArrayMap<>(mActiveUidIfaces), vpnArray, currentTime);
migrateTunTraffic(uidSnapshot, vpnArray);
}
/**
* Updates {@link #mTunAdjustedStats} with the delta containing traffic migrated off of VPNs.
*/
@GuardedBy("mStatsLock")
private void migrateTunTraffic(NetworkStats uidDetailStats, VpnInfo[] vpnInfoArray) {
if (mTunAdjustedStats == null) {
// Either device booted or system server restarted, hence traffic cannot be migrated
// correctly without knowing the past state of VPN's underlying networks.
mTunAdjustedStats = uidDetailStats;
mLastUidDetailSnapshot = uidDetailStats;
return;
}
// Migrate delta traffic from VPN to other apps.
NetworkStats delta = uidDetailStats.subtract(mLastUidDetailSnapshot);
for (VpnInfo info : vpnInfoArray) {
delta.migrateTun(info.ownerUid, info.vpnIface, info.underlyingIfaces);
}
// Filter out debug entries as that may lead to over counting.
delta.filterDebugEntries();
// Update #mTunAdjustedStats with migrated delta.
mTunAdjustedStats.combineAllValues(delta);
mTunAdjustedStats.setElapsedRealtime(uidDetailStats.getElapsedRealtime());
// Update last snapshot.
mLastUidDetailSnapshot = uidDetailStats;
}
/**

View File

@@ -812,37 +812,6 @@ public class NetworkStatsTest {
assertEquals(entry2, stats.getValues(1, null));
}
@Test
public void testFilterDebugEntries() {
NetworkStats.Entry entry1 = new NetworkStats.Entry(
"test1", 10100, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
DEFAULT_NETWORK_NO, 50000L, 25L, 100000L, 50L, 0L);
NetworkStats.Entry entry2 = new NetworkStats.Entry(
"test2", 10101, SET_DBG_VPN_IN, TAG_NONE, METERED_NO, ROAMING_NO,
DEFAULT_NETWORK_NO, 50000L, 25L, 100000L, 50L, 0L);
NetworkStats.Entry entry3 = new NetworkStats.Entry(
"test2", 10101, SET_DEFAULT, TAG_NONE, METERED_NO, ROAMING_NO,
DEFAULT_NETWORK_NO, 50000L, 25L, 100000L, 50L, 0L);
NetworkStats.Entry entry4 = new NetworkStats.Entry(
"test2", 10101, SET_DBG_VPN_OUT, TAG_NONE, METERED_NO, ROAMING_NO,
DEFAULT_NETWORK_NO, 50000L, 25L, 100000L, 50L, 0L);
NetworkStats stats = new NetworkStats(TEST_START, 4)
.addValues(entry1)
.addValues(entry2)
.addValues(entry3)
.addValues(entry4);
stats.filterDebugEntries();
assertEquals(2, stats.size());
assertEquals(entry1, stats.getValues(0, null));
assertEquals(entry3, stats.getValues(1, null));
}
@Test
public void testApply464xlatAdjustments() {
final String v4Iface = "v4-wlan0";

View File

@@ -57,7 +57,6 @@ import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_PO
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
@@ -217,16 +216,11 @@ public class NetworkStatsServiceTest {
expectNetworkStatsUidDetail(buildEmptyStats());
expectSystemReady();
assertNull(mService.getTunAdjustedStats());
mService.systemReady();
// Verify that system ready fetches realtime stats and initializes tun adjusted stats.
verify(mNetManager).getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL);
assertNotNull("failed to initialize TUN adjusted stats", mService.getTunAdjustedStats());
assertEquals(0, mService.getTunAdjustedStats().size());
mSession = mService.openSession();
assertNotNull("openSession() failed", mSession);
// catch INetworkManagementEventObserver during systemReady()
ArgumentCaptor<INetworkManagementEventObserver> networkObserver =
ArgumentCaptor.forClass(INetworkManagementEventObserver.class);
@@ -739,13 +733,11 @@ public class NetworkStatsServiceTest {
NetworkStats stats = mService.getDetailedUidStats(ifaceFilter);
// mNetManager#getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL) has following invocations:
// 1) NetworkStatsService#systemReady from #setUp.
// 2) mService#forceUpdateIfaces in the test above.
// 3) Finally, mService#getDetailedUidStats.
verify(mNetManager, times(3)).getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL);
assertTrue(ArrayUtils.contains(stats.getUniqueIfaces(), TEST_IFACE));
assertTrue(ArrayUtils.contains(stats.getUniqueIfaces(), stackedIface));
verify(mNetManager, times(1)).getNetworkStatsUidDetail(eq(UID_ALL), argThat(ifaces ->
ifaces != null && ifaces.length == 2
&& ArrayUtils.contains(ifaces, TEST_IFACE)
&& ArrayUtils.contains(ifaces, stackedIface)));
assertEquals(2, stats.size());
assertEquals(uidStats, stats.getValues(0, null));
assertEquals(tetheredStats1, stats.getValues(1, null));
@@ -1165,134 +1157,6 @@ public class NetworkStatsServiceTest {
assertUidTotal(buildTemplateMobileWildcard(), UID_VPN, 1100L, 100L, 1100L, 100L, 1);
}
@Test
public void recordSnapshot_migratesTunTrafficAndUpdatesTunAdjustedStats() throws Exception {
assertEquals(0, mService.getTunAdjustedStats().size());
// VPN using WiFi (TEST_IFACE).
VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
expectBandwidthControlCheck();
// create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption
// overhead per packet):
// 1000 bytes (100 packets) were downloaded by UID_RED over VPN.
// VPN received 1100 bytes (100 packets) over WiFi.
incrementCurrentTime(HOUR_IN_MILLIS);
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 2)
.addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 0L, 0L, 0L)
.addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1100L, 100L, 0L, 0L, 0L));
// this should lead to NSS#recordSnapshotLocked
mService.forceUpdateIfaces(
new Network[0], vpnInfos, new NetworkState[0], null /* activeIface */);
// Verify TUN adjusted stats have traffic migrated correctly.
// Of 1100 bytes VPN received over WiFi, expect 1000 bytes attributed to UID_RED and 100
// bytes attributed to UID_VPN.
NetworkStats tunAdjStats = mService.getTunAdjustedStats();
assertValues(
tunAdjStats, TEST_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 1000L, 100L, 0L, 0L, 0);
assertValues(
tunAdjStats, TEST_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 100L, 0L, 0L, 0L, 0);
}
@Test
public void getDetailedUidStats_migratesTunTrafficAndUpdatesTunAdjustedStats()
throws Exception {
assertEquals(0, mService.getTunAdjustedStats().size());
// VPN using WiFi (TEST_IFACE).
VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
expectBandwidthControlCheck();
mService.forceUpdateIfaces(
new Network[0], vpnInfos, new NetworkState[0], null /* activeIface */);
// create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption
// overhead per packet):
// 1000 bytes (100 packets) were downloaded by UID_RED over VPN.
// VPN received 1100 bytes (100 packets) over WiFi.
incrementCurrentTime(HOUR_IN_MILLIS);
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 2)
.addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 0L, 0L, 0L)
.addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1100L, 100L, 0L, 0L, 0L));
mService.getDetailedUidStats(INTERFACES_ALL);
// Verify internally maintained TUN adjusted stats
NetworkStats tunAdjStats = mService.getTunAdjustedStats();
// Verify stats for TEST_IFACE (WiFi):
// Of 1100 bytes VPN received over WiFi, expect 1000 bytes attributed to UID_RED and 100
// bytes attributed to UID_VPN.
assertValues(
tunAdjStats, TEST_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 1000L, 100L, 0L, 0L, 0);
assertValues(
tunAdjStats, TEST_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 100L, 0L, 0L, 0L, 0);
// Verify stats for TUN_IFACE; only UID_RED should have usage on it.
assertValues(
tunAdjStats, TUN_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 1000L, 100L, 0L, 0L, 0);
assertValues(
tunAdjStats, TUN_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 0L, 0L, 0L, 0L, 0);
// lets assume that since last time, VPN received another 1100 bytes (same assumptions as
// before i.e. UID_RED downloaded another 1000 bytes).
incrementCurrentTime(HOUR_IN_MILLIS);
// Note - NetworkStatsFactory returns counters that are monotonically increasing.
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 2)
.addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 2000L, 200L, 0L, 0L, 0L)
.addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 2200L, 200L, 0L, 0L, 0L));
mService.getDetailedUidStats(INTERFACES_ALL);
tunAdjStats = mService.getTunAdjustedStats();
// verify TEST_IFACE stats:
assertValues(
tunAdjStats, TEST_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 2000L, 200L, 0L, 0L, 0);
assertValues(
tunAdjStats, TEST_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 200L, 0L, 0L, 0L, 0);
// verify TUN_IFACE stats:
assertValues(
tunAdjStats, TUN_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 2000L, 200L, 0L, 0L, 0);
assertValues(
tunAdjStats, TUN_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 0L, 0L, 0L, 0L, 0);
}
@Test
public void getDetailedUidStats_returnsCorrectStatsWithVpnRunning() throws Exception {
// VPN using WiFi (TEST_IFACE).
VpnInfo[] vpnInfos = new VpnInfo[] {createVpnInfo(new String[] {TEST_IFACE})};
expectBandwidthControlCheck();
mService.forceUpdateIfaces(
new Network[0], vpnInfos, new NetworkState[0], null /* activeIface */);
// create some traffic (assume 10 bytes of MTU for VPN interface and 1 byte encryption
// overhead per packet):
// 1000 bytes (100 packets) were downloaded by UID_RED over VPN.
// VPN received 1100 bytes (100 packets) over WiFi.
incrementCurrentTime(HOUR_IN_MILLIS);
expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 2)
.addValues(TUN_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1000L, 100L, 0L, 0L, 0L)
.addValues(TEST_IFACE, UID_VPN, SET_DEFAULT, TAG_NONE, 1100L, 100L, 0L, 0L, 0L));
// Query realtime stats for TEST_IFACE.
NetworkStats queriedStats =
mService.getDetailedUidStats(new String[] {TEST_IFACE});
assertEquals(HOUR_IN_MILLIS, queriedStats.getElapsedRealtime());
// verify that returned stats are only for TEST_IFACE and VPN traffic is migrated correctly.
assertEquals(new String[] {TEST_IFACE}, queriedStats.getUniqueIfaces());
assertValues(
queriedStats, TEST_IFACE, UID_RED, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 1000L, 100L, 0L, 0L, 0);
assertValues(
queriedStats, TEST_IFACE, UID_VPN, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL,
DEFAULT_NETWORK_ALL, 100L, 0L, 0L, 0L, 0);
}
@Test
public void testRegisterUsageCallback() throws Exception {
// pretend that wifi network comes online; service should ask about full