NetworkStatsFactory: Take VPNs into account for network/battery stats
This change fixes detailed UID stats to ensure network and battery stats
both take VPNs into account. NetworkStatsFactory is being made aware of
VPNs enabled, and the full set of underlying networks present.
Since traffic can only be migrated over a NetworkStats delta, NSF
maintains a NetworkStats snapshot across all UIDs/ifaces/tags.
This snapshot gets updated whenever NSF records a new snapshot
(based on various hooks such as VPN updating its underlying networks,
network getting lost, etc.), or NetworkStatsService's
getDetailedUidStats() method being called.
This change widens the scope of the existing mPersistentSnapshot lock,
renaming it to mPersistentDataLock, and ensures that TUN migrations are
not done in parallel. Additionally, mVpnInfos is updated via
pointer-swapping, to reduce the scope of the mPersistentDataLock.
The safety of this change is predicated on:
1. NetworkStatsFactory lock not held, so services cannot deadlock through
the cyclical lock.
2. The broadening of the scope of the lock in NetworkStatsFactory has no
threading implications, as it is always the last (leaf node) lock held,
and therefore is impossible to have lock inversion.
Additionally, to ensure VPNs work with 464xlat, the VPN info passed to
the NetworkStatsFactory includes all underlying interfaces, instead of
only passing the first one.
This (partially) re-applies changes from:
aosp/972848: Add one more test for VPN usage stats.
aosp/972847: Addressing comments for http://ag/7700679.
aosp/885338: NetworkStatsService: Fix getDetailedUidStats to take VPNs
into account.
Co-developed with: Varun Anand <vaanand@google.com>
Bug: 113122541
Bug: 120145746
Bug: 129264869
Bug: 134244752
Test: FrameworksNetTest passing
Test: Manual tests show data usage fixes maintained.
Change-Id: I6466ec1411fc5ed6954125d27d353b6cd1be719e
diff --git a/core/java/android/net/INetworkStatsService.aidl b/core/java/android/net/INetworkStatsService.aidl
index 41efc50..e5f3d26 100644
--- a/core/java/android/net/INetworkStatsService.aidl
+++ b/core/java/android/net/INetworkStatsService.aidl
@@ -66,7 +66,6 @@
/** Force update of ifaces. */
void forceUpdateIfaces(
in Network[] defaultNetworks,
- in VpnInfo[] vpnArray,
in NetworkState[] networkStates,
in String activeIface);
/** Force update of statistics. */
diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java
index 9e79606..14a0cbf 100644
--- a/core/java/android/net/NetworkStats.java
+++ b/core/java/android/net/NetworkStats.java
@@ -23,7 +23,6 @@
import android.os.Parcel;
import android.os.Parcelable;
import android.os.SystemClock;
-import android.util.Slog;
import android.util.SparseBooleanArray;
import com.android.internal.annotations.VisibleForTesting;
@@ -37,6 +36,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
+import java.util.function.Predicate;
/**
* Collection of active network statistics. Can contain summary details across
@@ -994,23 +994,33 @@
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);
- 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);
+ if (predicate.test(entry)) {
+ if (nextOutputEntry != i) {
+ setValues(nextOutputEntry, entry);
+ }
nextOutputEntry++;
}
}
-
size = nextOutputEntry;
}
@@ -1289,7 +1299,8 @@
}
final Entry tmpEntry = new Entry();
- for (int i = 0; i < size; i++) {
+ final int origSize = size;
+ for (int i = 0; i < origSize; i++) {
if (!Objects.equals(iface[i], tunIface)) {
// Consider only entries that go onto the VPN interface.
continue;
@@ -1305,8 +1316,9 @@
tmpEntry.roaming = roaming[i];
tmpEntry.defaultNetwork = defaultNetwork[i];
- // In a first pass, compute each UID's total share of data across all underlyingIfaces.
- // This is computed on the basis of the share of each UID's usage over tunIface.
+ // In a first pass, compute this entry's total share of data across all
+ // underlyingIfaces. This is computed on the basis of the share of this entry's usage
+ // over tunIface.
// TODO: Consider refactoring first pass into a separate helper method.
long totalRxBytes = 0;
if (tunIfaceTotal.rxBytes > 0) {
@@ -1383,9 +1395,11 @@
* perInterfaceTotal[j].operations
/ underlyingIfacesTotal.operations;
}
-
+ // tmpEntry now contains the migrated data of the i-th entry for the j-th underlying
+ // interface. Add that data usage to this object.
combineValues(tmpEntry);
if (tag[i] == TAG_NONE) {
+ // Add the migrated data to moved so it is deducted from the VPN app later.
moved[j].add(tmpEntry);
// Add debug info
tmpEntry.set = SET_DBG_VPN_IN;
@@ -1401,8 +1415,8 @@
@NonNull String[] underlyingIfaces,
@NonNull Entry[] moved) {
for (int i = 0; i < underlyingIfaces.length; i++) {
- // Add debug info
moved[i].uid = tunUid;
+ // Add debug info
moved[i].set = SET_DBG_VPN_OUT;
moved[i].tag = TAG_NONE;
moved[i].iface = underlyingIfaces[i];
diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java
index c118bda..2d3c66d 100644
--- a/services/core/java/com/android/server/net/NetworkStatsFactory.java
+++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java
@@ -34,6 +34,7 @@
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.net.VpnInfo;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.ProcFileReader;
@@ -71,11 +72,25 @@
private INetd mNetdService;
- // A persistent Snapshot since device start for eBPF stats
- @GuardedBy("mPersistSnapshot")
- private final NetworkStats mPersistSnapshot;
+ /**
+ * Guards persistent data access in this class
+ *
+ * <p>In order to prevent deadlocks, critical sections protected by this lock SHALL NOT call out
+ * to other code that will acquire other locks within the system server. See b/134244752.
+ */
+ private static final Object sPersistentDataLock = new Object();
- // TODO: only do adjustments in NetworkStatsService and remove this.
+ /** Set containing info about active VPNs and their underlying networks. */
+ private static volatile VpnInfo[] sVpnInfos = new VpnInfo[0];
+
+ // A persistent snapshot of cumulative stats since device start
+ @GuardedBy("sPersistentDataLock")
+ private NetworkStats mPersistSnapshot;
+
+ // The persistent snapshot of tun and 464xlat adjusted stats since device start
+ @GuardedBy("sPersistentDataLock")
+ private NetworkStats mTunAnd464xlatAdjustedStats;
+
/**
* (Stacked interface) -> (base interface) association for all connected ifaces since boot.
*
@@ -92,6 +107,24 @@
}
/**
+ * Set active VPN information for data usage migration purposes
+ *
+ * <p>Traffic on TUN-based VPNs inherently all appear to be originated from the VPN providing
+ * app's UID. This method is used to support migration of VPN data usage, ensuring data is
+ * accurately billed to the real owner of the traffic.
+ *
+ * @param vpnArray The snapshot of the currently-running VPNs.
+ */
+ public static void updateVpnInfos(VpnInfo[] vpnArray) {
+ sVpnInfos = vpnArray.clone();
+ }
+
+ @VisibleForTesting
+ public static VpnInfo[] getVpnInfos() {
+ return sVpnInfos.clone();
+ }
+
+ /**
* Get a set of interfaces containing specified ifaces and stacked interfaces.
*
* <p>The added stacked interfaces are ifaces stacked on top of the specified ones, or ifaces
@@ -147,6 +180,7 @@
mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats");
mUseBpfStats = useBpfStats;
mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1);
+ mTunAnd464xlatAdjustedStats = new NetworkStats(SystemClock.elapsedRealtime(), -1);
}
public NetworkStats readBpfNetworkStatsDev() throws IOException {
@@ -279,16 +313,10 @@
*/
public NetworkStats readNetworkStatsDetail(
int limitUid, @Nullable String[] limitIfaces, int limitTag) throws IOException {
- final NetworkStats stats = readNetworkStatsDetailInternal(limitUid, limitIfaces, limitTag);
-
- // No locking here: apply464xlatAdjustments behaves fine with an add-only ConcurrentHashMap.
- // TODO: remove this and only apply adjustments in NetworkStatsService.
- stats.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats);
-
- return stats;
+ return readNetworkStatsDetailInternal(limitUid, limitIfaces, limitTag);
}
- @GuardedBy("mPersistSnapshot")
+ @GuardedBy("sPersistentDataLock")
private void requestSwapActiveStatsMapLocked() throws RemoteException {
// Ask netd to do a active map stats swap. When the binder call successfully returns,
// the system server should be able to safely read and clean the inactive map
@@ -303,11 +331,18 @@
private NetworkStats readNetworkStatsDetailInternal(
int limitUid, String[] limitIfaces, int limitTag) throws IOException {
- if (USE_NATIVE_PARSING) {
- final NetworkStats stats =
- new NetworkStats(SystemClock.elapsedRealtime(), 0 /* initialSize */);
- if (mUseBpfStats) {
- synchronized (mPersistSnapshot) {
+ // In order to prevent deadlocks, anything protected by this lock MUST NOT call out to other
+ // code that will acquire other locks within the system server. See b/134244752.
+ synchronized (sPersistentDataLock) {
+ // Take a reference. If this gets swapped out, we still have the old reference.
+ final VpnInfo[] vpnArray = sVpnInfos;
+ // Take a defensive copy. mPersistSnapshot is mutated in some cases below
+ final NetworkStats prev = mPersistSnapshot.clone();
+
+ if (USE_NATIVE_PARSING) {
+ final NetworkStats stats =
+ new NetworkStats(SystemClock.elapsedRealtime(), 0 /* initialSize */);
+ if (mUseBpfStats) {
try {
requestSwapActiveStatsMapLocked();
} catch (RemoteException e) {
@@ -316,32 +351,66 @@
// Stats are always read from the inactive map, so they must be read after the
// swap
if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), UID_ALL,
- null, TAG_ALL, mUseBpfStats) != 0) {
+ INTERFACES_ALL, TAG_ALL, mUseBpfStats) != 0) {
throw new IOException("Failed to parse network stats");
}
+
+ // BPF stats are incremental; fold into mPersistSnapshot.
mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
mPersistSnapshot.combineAllValues(stats);
- NetworkStats result = mPersistSnapshot.clone();
- result.filter(limitUid, limitIfaces, limitTag);
- return result;
+ } else {
+ if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), UID_ALL,
+ INTERFACES_ALL, TAG_ALL, mUseBpfStats) != 0) {
+ throw new IOException("Failed to parse network stats");
+ }
+ if (SANITY_CHECK_NATIVE) {
+ final NetworkStats javaStats = javaReadNetworkStatsDetail(mStatsXtUid,
+ UID_ALL, INTERFACES_ALL, TAG_ALL);
+ assertEquals(javaStats, stats);
+ }
+
+ mPersistSnapshot = stats;
}
} else {
- if (nativeReadNetworkStatsDetail(stats, mStatsXtUid.getAbsolutePath(), limitUid,
- limitIfaces, limitTag, mUseBpfStats) != 0) {
- throw new IOException("Failed to parse network stats");
- }
- if (SANITY_CHECK_NATIVE) {
- final NetworkStats javaStats = javaReadNetworkStatsDetail(mStatsXtUid, limitUid,
- limitIfaces, limitTag);
- assertEquals(javaStats, stats);
- }
- return stats;
+ mPersistSnapshot = javaReadNetworkStatsDetail(mStatsXtUid, UID_ALL, INTERFACES_ALL,
+ TAG_ALL);
}
- } else {
- return javaReadNetworkStatsDetail(mStatsXtUid, limitUid, limitIfaces, limitTag);
+
+ NetworkStats adjustedStats = adjustForTunAnd464Xlat(mPersistSnapshot, prev, vpnArray);
+
+ // Filter return values
+ adjustedStats.filter(limitUid, limitIfaces, limitTag);
+ return adjustedStats;
}
}
+ @GuardedBy("sPersistentDataLock")
+ private NetworkStats adjustForTunAnd464Xlat(
+ NetworkStats uidDetailStats, NetworkStats previousStats, VpnInfo[] vpnArray) {
+ // Calculate delta from last snapshot
+ final NetworkStats delta = uidDetailStats.subtract(previousStats);
+
+ // Apply 464xlat adjustments before VPN adjustments. If VPNs are using v4 on a v6 only
+ // network, the overhead is their fault.
+ // No locking here: apply464xlatAdjustments behaves fine with an add-only
+ // ConcurrentHashMap.
+ delta.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats);
+
+ // Migrate data usage over a VPN to the TUN network.
+ for (VpnInfo info : vpnArray) {
+ delta.migrateTun(info.ownerUid, info.vpnIface, info.underlyingIfaces);
+ }
+
+ // Filter out debug entries as that may lead to over counting.
+ delta.filterDebugEntries();
+
+ // Update mTunAnd464xlatAdjustedStats with migrated delta.
+ mTunAnd464xlatAdjustedStats.combineAllValues(delta);
+ mTunAnd464xlatAdjustedStats.setElapsedRealtime(uidDetailStats.getElapsedRealtime());
+
+ return mTunAnd464xlatAdjustedStats.clone();
+ }
+
/**
* Parse and return {@link NetworkStats} with UID-level details. Values are
* expected to monotonically increase since device boot.
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index f9972dd..6ca3e38 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -130,7 +130,6 @@
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.net.VpnInfo;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.FileRotator;
@@ -266,10 +265,6 @@
@GuardedBy("mStatsLock")
private Network[] mDefaultNetworks = new Network[0];
- /** Set containing info about active VPNs and their underlying networks. */
- @GuardedBy("mStatsLock")
- private VpnInfo[] mVpnInfos = new VpnInfo[0];
-
private final DropBoxNonMonotonicObserver mNonMonotonicObserver =
new DropBoxNonMonotonicObserver();
@@ -857,7 +852,6 @@
@Override
public void forceUpdateIfaces(
Network[] defaultNetworks,
- VpnInfo[] vpnArray,
NetworkState[] networkStates,
String activeIface) {
checkNetworkStackPermission(mContext);
@@ -865,7 +859,7 @@
final long token = Binder.clearCallingIdentity();
try {
- updateIfaces(defaultNetworks, vpnArray, networkStates, activeIface);
+ updateIfaces(defaultNetworks, networkStates, activeIface);
} finally {
Binder.restoreCallingIdentity(token);
}
@@ -1131,13 +1125,11 @@
private void updateIfaces(
Network[] defaultNetworks,
- VpnInfo[] vpnArray,
NetworkState[] networkStates,
String activeIface) {
synchronized (mStatsLock) {
mWakeLock.acquire();
try {
- mVpnInfos = vpnArray;
mActiveIface = activeIface;
updateIfacesLocked(defaultNetworks, networkStates);
} finally {
@@ -1147,10 +1139,9 @@
}
/**
- * Inspect all current {@link NetworkState} to derive mapping from {@code
- * iface} to {@link NetworkStatsHistory}. When multiple {@link NetworkInfo}
- * are active on a single {@code iface}, they are combined under a single
- * {@link NetworkIdentitySet}.
+ * Inspect all current {@link NetworkState} to derive mapping from {@code iface} to {@link
+ * NetworkStatsHistory}. When multiple {@link NetworkInfo} are active on a single {@code iface},
+ * they are combined under a single {@link NetworkIdentitySet}.
*/
@GuardedBy("mStatsLock")
private void updateIfacesLocked(Network[] defaultNetworks, NetworkState[] states) {
@@ -1276,18 +1267,19 @@
Trace.traceEnd(TRACE_TAG_NETWORK);
// For per-UID stats, pass the VPN info so VPN traffic is reattributed to responsible apps.
- VpnInfo[] vpnArray = mVpnInfos;
Trace.traceBegin(TRACE_TAG_NETWORK, "recordUid");
- mUidRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime);
+ mUidRecorder.recordSnapshotLocked(
+ uidSnapshot, mActiveUidIfaces, null /* vpnArray */, currentTime);
Trace.traceEnd(TRACE_TAG_NETWORK);
Trace.traceBegin(TRACE_TAG_NETWORK, "recordUidTag");
- mUidTagRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime);
+ mUidTagRecorder.recordSnapshotLocked(
+ uidSnapshot, mActiveUidIfaces, null /* vpnArray */, currentTime);
Trace.traceEnd(TRACE_TAG_NETWORK);
// We need to make copies of member fields that are sent to the observer to avoid
// a race condition between the service handler thread and the observer's
mStatsObservers.updateStats(xtSnapshot, uidSnapshot, new ArrayMap<>(mActiveIfaces),
- new ArrayMap<>(mActiveUidIfaces), vpnArray, currentTime);
+ new ArrayMap<>(mActiveUidIfaces), null /* vpnArray */, currentTime);
}
/**
@@ -1660,8 +1652,6 @@
*/
private NetworkStats getNetworkStatsUidDetail(String[] ifaces)
throws RemoteException {
-
- // TODO: remove 464xlat adjustments from NetworkStatsFactory and apply all at once here.
final NetworkStats uidSnapshot = mNetworkManager.getNetworkStatsUidDetail(UID_ALL,
ifaces);