Remove qtaguid support in NetworkStatsFactory
All devices shipped with S+ release would have 4.9+ kernel
with bpf enabled. And for older devices qtag uid is
already broken for a long time. Hence, it is safe to
remove the related code.
Bug: 234315786
Test: atest FrameworksNetTests CtsNetTestCases \
ConnectivityCoverageTests
Change-Id: Id2a6bfdf32793e5f48eb74da3b662906e5ed380e
diff --git a/service-t/src/com/android/server/net/NetworkStatsFactory.java b/service-t/src/com/android/server/net/NetworkStatsFactory.java
index 8aa3dbc..4a6741c 100644
--- a/service-t/src/com/android/server/net/NetworkStatsFactory.java
+++ b/service-t/src/com/android/server/net/NetworkStatsFactory.java
@@ -17,9 +17,7 @@
package com.android.server.net;
import static android.net.NetworkStats.INTERFACES_ALL;
-import static android.net.NetworkStats.SET_ALL;
import static android.net.NetworkStats.TAG_ALL;
-import static android.net.NetworkStats.TAG_NONE;
import static android.net.NetworkStats.UID_ALL;
import android.annotation.NonNull;
@@ -28,19 +26,12 @@
import android.net.NetworkStats;
import android.net.UnderlyingNetworkInfo;
import android.os.ServiceSpecificException;
-import android.os.StrictMode;
import android.os.SystemClock;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.util.ProcFileReader;
-import com.android.net.module.util.CollectionUtils;
import com.android.server.BpfNetMaps;
-import libcore.io.IoUtils;
-
-import java.io.File;
-import java.io.FileInputStream;
import java.io.IOException;
import java.net.ProtocolException;
import java.util.Arrays;
@@ -61,18 +52,6 @@
private static final String TAG = "NetworkStatsFactory";
- private static final boolean USE_NATIVE_PARSING = true;
- private static final boolean VALIDATE_NATIVE_STATS = false;
-
- /** Path to {@code /proc/net/xt_qtaguid/iface_stat_all}. */
- private final File mStatsXtIfaceAll;
- /** Path to {@code /proc/net/xt_qtaguid/iface_stat_fmt}. */
- private final File mStatsXtIfaceFmt;
- /** Path to {@code /proc/net/xt_qtaguid/stats}. */
- private final File mStatsXtUid;
-
- private final boolean mUseBpfStats;
-
private final Context mContext;
private final BpfNetMaps mBpfNetMaps;
@@ -107,12 +86,13 @@
* are expected to monotonically increase since device boot.
*/
@NonNull
- public NetworkStats getNetworkStatsDetail(@Nullable String path, int limitUid,
- @Nullable String[] limitIfaces, int limitTag, boolean useBpfStats)
- throws IOException {
+ public NetworkStats getNetworkStatsDetail(int limitUid, @Nullable String[] limitIfaces,
+ int limitTag) throws IOException {
final NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 0);
- final int ret = nativeReadNetworkStatsDetail(stats, path,
- limitUid, limitIfaces, limitTag, useBpfStats);
+ // TODO: remove both path and useBpfStats arguments.
+ // The path is never used if useBpfStats is true.
+ final int ret = nativeReadNetworkStatsDetail(stats, null /* path */,
+ limitUid, limitIfaces, limitTag, true /* useBpfStats */);
if (ret != 0) {
throw new IOException("Failed to parse network stats");
}
@@ -206,16 +186,11 @@
}
public NetworkStatsFactory(@NonNull Context ctx) {
- this(ctx, new File("/proc/"), true, new Dependencies());
+ this(ctx, new Dependencies());
}
@VisibleForTesting
- public NetworkStatsFactory(@NonNull Context ctx, File procRoot, boolean useBpfStats,
- Dependencies deps) {
- mStatsXtIfaceAll = new File(procRoot, "net/xt_qtaguid/iface_stat_all");
- mStatsXtIfaceFmt = new File(procRoot, "net/xt_qtaguid/iface_stat_fmt");
- mStatsXtUid = new File(procRoot, "net/xt_qtaguid/stats");
- mUseBpfStats = useBpfStats;
+ public NetworkStatsFactory(@NonNull Context ctx, Dependencies deps) {
mBpfNetMaps = deps.createBpfNetMaps(ctx);
synchronized (mPersistentDataLock) {
mPersistSnapshot = new NetworkStats(SystemClock.elapsedRealtime(), -1);
@@ -230,104 +205,18 @@
* using {@code /proc/net/dev} style hooks, which may include non IP layer
* traffic. Values monotonically increase since device boot, and may include
* details about inactive interfaces.
- *
- * @throws IllegalStateException when problem parsing stats.
*/
public NetworkStats readNetworkStatsSummaryDev() throws IOException {
-
- // Return xt_bpf stats if switched to bpf module.
- if (mUseBpfStats) return mDeps.getNetworkStatsDev();
-
- final StrictMode.ThreadPolicy savedPolicy = StrictMode.allowThreadDiskReads();
-
- final NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 6);
- final NetworkStats.Entry entry = new NetworkStats.Entry();
-
- ProcFileReader reader = null;
- try {
- reader = new ProcFileReader(new FileInputStream(mStatsXtIfaceAll));
-
- while (reader.hasMoreData()) {
- entry.iface = reader.nextString();
- entry.uid = UID_ALL;
- entry.set = SET_ALL;
- entry.tag = TAG_NONE;
-
- final boolean active = reader.nextInt() != 0;
-
- // always include snapshot values
- entry.rxBytes = reader.nextLong();
- entry.rxPackets = reader.nextLong();
- entry.txBytes = reader.nextLong();
- entry.txPackets = reader.nextLong();
-
- // fold in active numbers, but only when active
- if (active) {
- entry.rxBytes += reader.nextLong();
- entry.rxPackets += reader.nextLong();
- entry.txBytes += reader.nextLong();
- entry.txPackets += reader.nextLong();
- }
-
- stats.insertEntry(entry);
- reader.finishLine();
- }
- } catch (NullPointerException|NumberFormatException e) {
- throw protocolExceptionWithCause("problem parsing stats", e);
- } finally {
- IoUtils.closeQuietly(reader);
- StrictMode.setThreadPolicy(savedPolicy);
- }
- return stats;
+ return mDeps.getNetworkStatsDev();
}
/**
* Parse and return interface-level summary {@link NetworkStats}. Designed
* to return only IP layer traffic. Values monotonically increase since
* device boot, and may include details about inactive interfaces.
- *
- * @throws IllegalStateException when problem parsing stats.
*/
public NetworkStats readNetworkStatsSummaryXt() throws IOException {
-
- // Return xt_bpf stats if qtaguid module is replaced.
- if (mUseBpfStats) return mDeps.getNetworkStatsDev();
-
- final StrictMode.ThreadPolicy savedPolicy = StrictMode.allowThreadDiskReads();
-
- // return null when kernel doesn't support
- if (!mStatsXtIfaceFmt.exists()) return null;
-
- final NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 6);
- final NetworkStats.Entry entry = new NetworkStats.Entry();
-
- ProcFileReader reader = null;
- try {
- // open and consume header line
- reader = new ProcFileReader(new FileInputStream(mStatsXtIfaceFmt));
- reader.finishLine();
-
- while (reader.hasMoreData()) {
- entry.iface = reader.nextString();
- entry.uid = UID_ALL;
- entry.set = SET_ALL;
- entry.tag = TAG_NONE;
-
- entry.rxBytes = reader.nextLong();
- entry.rxPackets = reader.nextLong();
- entry.txBytes = reader.nextLong();
- entry.txPackets = reader.nextLong();
-
- stats.insertEntry(entry);
- reader.finishLine();
- }
- } catch (NullPointerException|NumberFormatException e) {
- throw protocolExceptionWithCause("problem parsing stats", e);
- } finally {
- IoUtils.closeQuietly(reader);
- StrictMode.setThreadPolicy(savedPolicy);
- }
- return stats;
+ return mDeps.getNetworkStatsDev();
}
public NetworkStats readNetworkStatsDetail() throws IOException {
@@ -364,33 +253,14 @@
// Take a defensive copy. mPersistSnapshot is mutated in some cases below
final NetworkStats prev = mPersistSnapshot.clone();
- if (USE_NATIVE_PARSING) {
- if (mUseBpfStats) {
- requestSwapActiveStatsMapLocked();
- // Stats are always read from the inactive map, so they must be read after the
- // swap
- final NetworkStats stats = mDeps.getNetworkStatsDetail(null /* path */, UID_ALL,
- INTERFACES_ALL, TAG_ALL, true /* useBpfStats */);
-
- // BPF stats are incremental; fold into mPersistSnapshot.
- mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
- mPersistSnapshot.combineAllValues(stats);
- } else {
- final NetworkStats stats = mDeps.getNetworkStatsDetail(
- mStatsXtUid.getAbsolutePath(), UID_ALL,
- INTERFACES_ALL, TAG_ALL, false /* useBpfStats */);
- if (VALIDATE_NATIVE_STATS) {
- final NetworkStats javaStats = javaReadNetworkStatsDetail(mStatsXtUid,
- UID_ALL, INTERFACES_ALL, TAG_ALL);
- assertEquals(javaStats, stats);
- }
-
- mPersistSnapshot = stats;
- }
- } else {
- mPersistSnapshot = javaReadNetworkStatsDetail(mStatsXtUid, UID_ALL, INTERFACES_ALL,
- TAG_ALL);
- }
+ requestSwapActiveStatsMapLocked();
+ // Stats are always read from the inactive map, so they must be read after the
+ // swap
+ final NetworkStats stats = mDeps.getNetworkStatsDetail(
+ UID_ALL, INTERFACES_ALL, TAG_ALL);
+ // BPF stats are incremental; fold into mPersistSnapshot.
+ mPersistSnapshot.setElapsedRealtime(stats.getElapsedRealtime());
+ mPersistSnapshot.combineAllValues(stats);
NetworkStats adjustedStats = adjustForTunAnd464Xlat(mPersistSnapshot, prev, vpnArray);
@@ -427,62 +297,6 @@
return mTunAnd464xlatAdjustedStats.clone();
}
- /**
- * Parse and return {@link NetworkStats} with UID-level details. Values are
- * expected to monotonically increase since device boot.
- */
- @VisibleForTesting
- public static NetworkStats javaReadNetworkStatsDetail(File detailPath, int limitUid,
- String[] limitIfaces, int limitTag)
- throws IOException {
- final StrictMode.ThreadPolicy savedPolicy = StrictMode.allowThreadDiskReads();
-
- final NetworkStats stats = new NetworkStats(SystemClock.elapsedRealtime(), 24);
- final NetworkStats.Entry entry = new NetworkStats.Entry();
-
- int idx = 1;
- int lastIdx = 1;
-
- ProcFileReader reader = null;
- try {
- // open and consume header line
- reader = new ProcFileReader(new FileInputStream(detailPath));
- reader.finishLine();
-
- while (reader.hasMoreData()) {
- idx = reader.nextInt();
- if (idx != lastIdx + 1) {
- throw new ProtocolException(
- "inconsistent idx=" + idx + " after lastIdx=" + lastIdx);
- }
- lastIdx = idx;
-
- entry.iface = reader.nextString();
- entry.tag = kernelToTag(reader.nextString());
- entry.uid = reader.nextInt();
- entry.set = reader.nextInt();
- entry.rxBytes = reader.nextLong();
- entry.rxPackets = reader.nextLong();
- entry.txBytes = reader.nextLong();
- entry.txPackets = reader.nextLong();
-
- if ((limitIfaces == null || CollectionUtils.contains(limitIfaces, entry.iface))
- && (limitUid == UID_ALL || limitUid == entry.uid)
- && (limitTag == TAG_ALL || limitTag == entry.tag)) {
- stats.insertEntry(entry);
- }
-
- reader.finishLine();
- }
- } catch (NullPointerException|NumberFormatException e) {
- throw protocolExceptionWithCause("problem parsing idx " + idx, e);
- } finally {
- IoUtils.closeQuietly(reader);
- StrictMode.setThreadPolicy(savedPolicy);
- }
-
- return stats;
- }
public void assertEquals(NetworkStats expected, NetworkStats actual) {
if (expected.size() != actual.size()) {
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsFactoryTest.java b/tests/unit/java/com/android/server/net/NetworkStatsFactoryTest.java
index 8f47e1f..14455fa 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsFactoryTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsFactoryTest.java
@@ -34,7 +34,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.doReturn;
@@ -90,7 +89,7 @@
// related to networkStatsFactory is compiled to a minimal native library and loaded here.
System.loadLibrary("networkstatsfactorytestjni");
doReturn(mBpfNetMaps).when(mDeps).createBpfNetMaps(any());
- mFactory = new NetworkStatsFactory(mContext, mTestProc, true, mDeps);
+ mFactory = new NetworkStatsFactory(mContext, mDeps);
mFactory.updateUnderlyingNetworkInfos(new UnderlyingNetworkInfo[0]);
}
@@ -533,8 +532,8 @@
final NetworkStats statsFromResource = parseNetworkStatsFromGoldenSample(resourceId,
24 /* initialSize */, true /* consumeHeader */, false /* checkActive */,
true /* isUidData */);
- doReturn(statsFromResource).when(mDeps).getNetworkStatsDetail(any(), anyInt(), any(),
- anyInt(), anyBoolean());
+ doReturn(statsFromResource).when(mDeps).getNetworkStatsDetail(anyInt(), any(),
+ anyInt());
return mFactory.readNetworkStatsDetail();
}