Remove all static members from NetworkStatsFactory.
NetworkStatsFactory is owned by NetworkStatsService, and any
accesses to NSF data should go through NSS.
Test: atest FrameworksNetTests
Change-Id: Idbd0dbbaeb11313f63474e7ec0e01f974349fc89
diff --git a/core/java/android/net/INetworkStatsService.aidl b/core/java/android/net/INetworkStatsService.aidl
index e5f3d26..9994f9f 100644
--- a/core/java/android/net/INetworkStatsService.aidl
+++ b/core/java/android/net/INetworkStatsService.aidl
@@ -67,7 +67,8 @@
void forceUpdateIfaces(
in Network[] defaultNetworks,
in NetworkState[] networkStates,
- in String activeIface);
+ in String activeIface,
+ in VpnInfo[] vpnInfos);
/** Force update of statistics. */
@UnsupportedAppUsage
void forceUpdate();
diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java
index 9344ef7..3ca1803 100644
--- a/services/core/java/com/android/server/net/NetworkStatsFactory.java
+++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java
@@ -78,17 +78,17 @@
* <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();
+ private final Object mPersistentDataLock = new Object();
/** Set containing info about active VPNs and their underlying networks. */
- private static volatile VpnInfo[] sVpnInfos = new VpnInfo[0];
+ private volatile VpnInfo[] mVpnInfos = new VpnInfo[0];
// A persistent snapshot of cumulative stats since device start
- @GuardedBy("sPersistentDataLock")
+ @GuardedBy("mPersistentDataLock")
private NetworkStats mPersistSnapshot;
// The persistent snapshot of tun and 464xlat adjusted stats since device start
- @GuardedBy("sPersistentDataLock")
+ @GuardedBy("mPersistentDataLock")
private NetworkStats mTunAnd464xlatAdjustedStats;
/**
@@ -97,12 +97,13 @@
* Because counters must never roll backwards, once a given interface is stacked on top of an
* underlying interface, the stacked interface can never be stacked on top of
* another interface. */
- private static final ConcurrentHashMap<String, String> sStackedIfaces
+ private final ConcurrentHashMap<String, String> mStackedIfaces
= new ConcurrentHashMap<>();
- public static void noteStackedIface(String stackedIface, String baseIface) {
+ /** Informs the factory of a new stacked interface. */
+ public void noteStackedIface(String stackedIface, String baseIface) {
if (stackedIface != null && baseIface != null) {
- sStackedIfaces.put(stackedIface, baseIface);
+ mStackedIfaces.put(stackedIface, baseIface);
}
}
@@ -115,13 +116,8 @@
*
* @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();
+ public void updateVpnInfos(VpnInfo[] vpnArray) {
+ mVpnInfos = vpnArray.clone();
}
/**
@@ -132,7 +128,7 @@
* {@link #noteStackedIface(String, String)}, but only interfaces noted before this method
* is called are guaranteed to be included.
*/
- public static String[] augmentWithStackedInterfaces(@Nullable String[] requiredIfaces) {
+ public String[] augmentWithStackedInterfaces(@Nullable String[] requiredIfaces) {
if (requiredIfaces == NetworkStats.INTERFACES_ALL) {
return null;
}
@@ -142,7 +138,7 @@
// elements as they existed upon construction exactly once, and may
// (but are not guaranteed to) reflect any modifications subsequent to construction".
// This is enough here.
- for (Map.Entry<String, String> entry : sStackedIfaces.entrySet()) {
+ for (Map.Entry<String, String> entry : mStackedIfaces.entrySet()) {
if (relatedIfaces.contains(entry.getKey())) {
relatedIfaces.add(entry.getValue());
} else if (relatedIfaces.contains(entry.getValue())) {
@@ -158,17 +154,12 @@
* Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}.
* @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean)
*/
- public static void apply464xlatAdjustments(NetworkStats baseTraffic,
+ public void apply464xlatAdjustments(NetworkStats baseTraffic,
NetworkStats stackedTraffic, boolean useBpfStats) {
- NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces,
+ NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces,
useBpfStats);
}
- @VisibleForTesting
- public static void clearStackedIfaces() {
- sStackedIfaces.clear();
- }
-
public NetworkStatsFactory() {
this(new File("/proc/"), new File("/sys/fs/bpf/map_netd_app_uid_stats_map").exists());
}
@@ -179,7 +170,7 @@
mStatsXtIfaceFmt = new File(procRoot, "net/xt_qtaguid/iface_stat_fmt");
mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats");
mUseBpfStats = useBpfStats;
- synchronized (sPersistentDataLock) {
+ synchronized (mPersistentDataLock) {
mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1);
mTunAnd464xlatAdjustedStats = new NetworkStats(SystemClock.elapsedRealtime(), -1);
}
@@ -304,7 +295,7 @@
return readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL);
}
- @GuardedBy("sPersistentDataLock")
+ @GuardedBy("mPersistentDataLock")
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
@@ -328,9 +319,9 @@
int limitUid, String[] limitIfaces, int limitTag) throws IOException {
// 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) {
+ synchronized (mPersistentDataLock) {
// Take a reference. If this gets swapped out, we still have the old reference.
- final VpnInfo[] vpnArray = sVpnInfos;
+ final VpnInfo[] vpnArray = mVpnInfos;
// Take a defensive copy. mPersistSnapshot is mutated in some cases below
final NetworkStats prev = mPersistSnapshot.clone();
@@ -379,7 +370,7 @@
}
}
- @GuardedBy("sPersistentDataLock")
+ @GuardedBy("mPersistentDataLock")
private NetworkStats adjustForTunAnd464Xlat(
NetworkStats uidDetailStats, NetworkStats previousStats, VpnInfo[] vpnArray) {
// Calculate delta from last snapshot
@@ -389,7 +380,7 @@
// network, the overhead is their fault.
// No locking here: apply464xlatAdjustments behaves fine with an add-only
// ConcurrentHashMap.
- delta.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats);
+ delta.apply464xlatAdjustments(mStackedIfaces, mUseBpfStats);
// Migrate data usage over a VPN to the TUN network.
for (VpnInfo info : vpnArray) {
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index e67b391..4f4377d 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -130,6 +130,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.DumpUtils;
import com.android.internal.util.FileRotator;
@@ -777,7 +778,7 @@
public NetworkStats getDetailedUidStats(String[] requiredIfaces) {
try {
final String[] ifacesToQuery =
- NetworkStatsFactory.augmentWithStackedInterfaces(requiredIfaces);
+ mStatsFactory.augmentWithStackedInterfaces(requiredIfaces);
return getNetworkStatsUidDetail(ifacesToQuery);
} catch (RemoteException e) {
Log.wtf(TAG, "Error compiling UID stats", e);
@@ -829,7 +830,8 @@
public void forceUpdateIfaces(
Network[] defaultNetworks,
NetworkState[] networkStates,
- String activeIface) {
+ String activeIface,
+ VpnInfo[] vpnInfos) {
checkNetworkStackPermission(mContext);
final long token = Binder.clearCallingIdentity();
@@ -838,6 +840,11 @@
} finally {
Binder.restoreCallingIdentity(token);
}
+
+ // Update the VPN underlying interfaces only after the poll is made and tun data has been
+ // migrated. Otherwise the migration would use the new interfaces instead of the ones that
+ // were current when the polled data was transferred.
+ mStatsFactory.updateVpnInfos(vpnInfos);
}
@Override
@@ -1649,7 +1656,7 @@
// fold tethering stats and operations into uid snapshot
final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID);
tetherSnapshot.filter(UID_ALL, ifaces, TAG_ALL);
- NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot,
+ mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot,
mUseBpfTrafficStats);
uidSnapshot.combineAllValues(tetherSnapshot);
@@ -1660,7 +1667,7 @@
final NetworkStats vtStats = telephonyManager.getVtDataUsage(STATS_PER_UID);
if (vtStats != null) {
vtStats.filter(UID_ALL, ifaces, TAG_ALL);
- NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats,
+ mStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats,
mUseBpfTrafficStats);
uidSnapshot.combineAllValues(vtStats);
}
