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
This commit is contained in:
Lorenzo Colitti
2019-06-24 13:50:45 +09:00
parent 8b6fdeca3f
commit bcaf1f959b
3 changed files with 33 additions and 34 deletions

View File

@@ -67,7 +67,8 @@ interface INetworkStatsService {
void forceUpdateIfaces( void forceUpdateIfaces(
in Network[] defaultNetworks, in Network[] defaultNetworks,
in NetworkState[] networkStates, in NetworkState[] networkStates,
in String activeIface); in String activeIface,
in VpnInfo[] vpnInfos);
/** Force update of statistics. */ /** Force update of statistics. */
@UnsupportedAppUsage @UnsupportedAppUsage
void forceUpdate(); void forceUpdate();

View File

@@ -78,17 +78,17 @@ public class NetworkStatsFactory {
* <p>In order to prevent deadlocks, critical sections protected by this lock SHALL NOT call out * <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. * 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. */ /** 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 // A persistent snapshot of cumulative stats since device start
@GuardedBy("sPersistentDataLock") @GuardedBy("mPersistentDataLock")
private NetworkStats mPersistSnapshot; private NetworkStats mPersistSnapshot;
// The persistent snapshot of tun and 464xlat adjusted stats since device start // The persistent snapshot of tun and 464xlat adjusted stats since device start
@GuardedBy("sPersistentDataLock") @GuardedBy("mPersistentDataLock")
private NetworkStats mTunAnd464xlatAdjustedStats; private NetworkStats mTunAnd464xlatAdjustedStats;
/** /**
@@ -97,12 +97,13 @@ public class NetworkStatsFactory {
* Because counters must never roll backwards, once a given interface is stacked on top of an * 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 * underlying interface, the stacked interface can never be stacked on top of
* another interface. */ * another interface. */
private static final ConcurrentHashMap<String, String> sStackedIfaces private final ConcurrentHashMap<String, String> mStackedIfaces
= new ConcurrentHashMap<>(); = 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) { if (stackedIface != null && baseIface != null) {
sStackedIfaces.put(stackedIface, baseIface); mStackedIfaces.put(stackedIface, baseIface);
} }
} }
@@ -115,13 +116,8 @@ public class NetworkStatsFactory {
* *
* @param vpnArray The snapshot of the currently-running VPNs. * @param vpnArray The snapshot of the currently-running VPNs.
*/ */
public static void updateVpnInfos(VpnInfo[] vpnArray) { public void updateVpnInfos(VpnInfo[] vpnArray) {
sVpnInfos = vpnArray.clone(); mVpnInfos = vpnArray.clone();
}
@VisibleForTesting
public static VpnInfo[] getVpnInfos() {
return sVpnInfos.clone();
} }
/** /**
@@ -132,7 +128,7 @@ public class NetworkStatsFactory {
* {@link #noteStackedIface(String, String)}, but only interfaces noted before this method * {@link #noteStackedIface(String, String)}, but only interfaces noted before this method
* is called are guaranteed to be included. * 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) { if (requiredIfaces == NetworkStats.INTERFACES_ALL) {
return null; return null;
} }
@@ -142,7 +138,7 @@ public class NetworkStatsFactory {
// elements as they existed upon construction exactly once, and may // elements as they existed upon construction exactly once, and may
// (but are not guaranteed to) reflect any modifications subsequent to construction". // (but are not guaranteed to) reflect any modifications subsequent to construction".
// This is enough here. // 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())) { if (relatedIfaces.contains(entry.getKey())) {
relatedIfaces.add(entry.getValue()); relatedIfaces.add(entry.getValue());
} else if (relatedIfaces.contains(entry.getValue())) { } else if (relatedIfaces.contains(entry.getValue())) {
@@ -158,17 +154,12 @@ public class NetworkStatsFactory {
* Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}. * Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}.
* @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean) * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean)
*/ */
public static void apply464xlatAdjustments(NetworkStats baseTraffic, public void apply464xlatAdjustments(NetworkStats baseTraffic,
NetworkStats stackedTraffic, boolean useBpfStats) { NetworkStats stackedTraffic, boolean useBpfStats) {
NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces, NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces,
useBpfStats); useBpfStats);
} }
@VisibleForTesting
public static void clearStackedIfaces() {
sStackedIfaces.clear();
}
public NetworkStatsFactory() { public NetworkStatsFactory() {
this(new File("/proc/"), new File("/sys/fs/bpf/map_netd_app_uid_stats_map").exists()); this(new File("/proc/"), new File("/sys/fs/bpf/map_netd_app_uid_stats_map").exists());
} }
@@ -179,7 +170,7 @@ public class NetworkStatsFactory {
mStatsXtIfaceFmt = new File(procRoot, "net/xt_qtaguid/iface_stat_fmt"); mStatsXtIfaceFmt = new File(procRoot, "net/xt_qtaguid/iface_stat_fmt");
mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats"); mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats");
mUseBpfStats = useBpfStats; mUseBpfStats = useBpfStats;
synchronized (sPersistentDataLock) { synchronized (mPersistentDataLock) {
mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1); mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1);
mTunAnd464xlatAdjustedStats = new NetworkStats(SystemClock.elapsedRealtime(), -1); mTunAnd464xlatAdjustedStats = new NetworkStats(SystemClock.elapsedRealtime(), -1);
} }
@@ -304,7 +295,7 @@ public class NetworkStatsFactory {
return readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL); return readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL);
} }
@GuardedBy("sPersistentDataLock") @GuardedBy("mPersistentDataLock")
private void requestSwapActiveStatsMapLocked() throws RemoteException { private void requestSwapActiveStatsMapLocked() throws RemoteException {
// Ask netd to do a active map stats swap. When the binder call successfully returns, // 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 // the system server should be able to safely read and clean the inactive map
@@ -328,9 +319,9 @@ public class NetworkStatsFactory {
int limitUid, String[] limitIfaces, int limitTag) throws IOException { int limitUid, String[] limitIfaces, int limitTag) throws IOException {
// In order to prevent deadlocks, anything protected by this lock MUST NOT call out to other // 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. // 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. // 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 // Take a defensive copy. mPersistSnapshot is mutated in some cases below
final NetworkStats prev = mPersistSnapshot.clone(); final NetworkStats prev = mPersistSnapshot.clone();
@@ -379,7 +370,7 @@ public class NetworkStatsFactory {
} }
} }
@GuardedBy("sPersistentDataLock") @GuardedBy("mPersistentDataLock")
private NetworkStats adjustForTunAnd464Xlat( private NetworkStats adjustForTunAnd464Xlat(
NetworkStats uidDetailStats, NetworkStats previousStats, VpnInfo[] vpnArray) { NetworkStats uidDetailStats, NetworkStats previousStats, VpnInfo[] vpnArray) {
// Calculate delta from last snapshot // Calculate delta from last snapshot
@@ -389,7 +380,7 @@ public class NetworkStatsFactory {
// network, the overhead is their fault. // network, the overhead is their fault.
// No locking here: apply464xlatAdjustments behaves fine with an add-only // No locking here: apply464xlatAdjustments behaves fine with an add-only
// ConcurrentHashMap. // ConcurrentHashMap.
delta.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats); delta.apply464xlatAdjustments(mStackedIfaces, mUseBpfStats);
// Migrate data usage over a VPN to the TUN network. // Migrate data usage over a VPN to the TUN network.
for (VpnInfo info : vpnArray) { for (VpnInfo info : vpnArray) {

View File

@@ -130,6 +130,7 @@ import android.util.proto.ProtoOutputStream;
import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.net.VpnInfo;
import com.android.internal.util.ArrayUtils; import com.android.internal.util.ArrayUtils;
import com.android.internal.util.DumpUtils; import com.android.internal.util.DumpUtils;
import com.android.internal.util.FileRotator; import com.android.internal.util.FileRotator;
@@ -777,7 +778,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
public NetworkStats getDetailedUidStats(String[] requiredIfaces) { public NetworkStats getDetailedUidStats(String[] requiredIfaces) {
try { try {
final String[] ifacesToQuery = final String[] ifacesToQuery =
NetworkStatsFactory.augmentWithStackedInterfaces(requiredIfaces); mStatsFactory.augmentWithStackedInterfaces(requiredIfaces);
return getNetworkStatsUidDetail(ifacesToQuery); return getNetworkStatsUidDetail(ifacesToQuery);
} catch (RemoteException e) { } catch (RemoteException e) {
Log.wtf(TAG, "Error compiling UID stats", e); Log.wtf(TAG, "Error compiling UID stats", e);
@@ -829,7 +830,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
public void forceUpdateIfaces( public void forceUpdateIfaces(
Network[] defaultNetworks, Network[] defaultNetworks,
NetworkState[] networkStates, NetworkState[] networkStates,
String activeIface) { String activeIface,
VpnInfo[] vpnInfos) {
checkNetworkStackPermission(mContext); checkNetworkStackPermission(mContext);
final long token = Binder.clearCallingIdentity(); final long token = Binder.clearCallingIdentity();
@@ -838,6 +840,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
} finally { } finally {
Binder.restoreCallingIdentity(token); 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 @Override
@@ -1649,7 +1656,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// fold tethering stats and operations into uid snapshot // fold tethering stats and operations into uid snapshot
final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID); final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID);
tetherSnapshot.filter(UID_ALL, ifaces, TAG_ALL); tetherSnapshot.filter(UID_ALL, ifaces, TAG_ALL);
NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot, mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot,
mUseBpfTrafficStats); mUseBpfTrafficStats);
uidSnapshot.combineAllValues(tetherSnapshot); uidSnapshot.combineAllValues(tetherSnapshot);
@@ -1660,7 +1667,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
final NetworkStats vtStats = telephonyManager.getVtDataUsage(STATS_PER_UID); final NetworkStats vtStats = telephonyManager.getVtDataUsage(STATS_PER_UID);
if (vtStats != null) { if (vtStats != null) {
vtStats.filter(UID_ALL, ifaces, TAG_ALL); vtStats.filter(UID_ALL, ifaces, TAG_ALL);
NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats, mStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats,
mUseBpfTrafficStats); mUseBpfTrafficStats);
uidSnapshot.combineAllValues(vtStats); uidSnapshot.combineAllValues(vtStats);
} }