Merge "Fix negative uid stats caused by 464xlat adjust when eBPF is on."
diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java
index e270fc2..5447f59 100644
--- a/core/java/android/net/NetworkStats.java
+++ b/core/java/android/net/NetworkStats.java
@@ -811,14 +811,19 @@
* packet needs to be subtracted from the root UID on the base interface both for tx
* and rx traffic (http://b/12249687, http:/b/33681750).
*
+ * As for eBPF, the per uid stats is collected by different hook, the rx packets on base
+ * interface will not be counted. Thus, the adjustment on root uid is only needed in tx
+ * direction.
+ *
* <p>This method will behave fine if {@code stackedIfaces} is an non-synchronized but add-only
* {@code ConcurrentHashMap}
* @param baseTraffic Traffic on the base interfaces. Will be mutated.
* @param stackedTraffic Stats with traffic stacked on top of our ifaces. Will also be mutated.
* @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both.
+ * @param useBpfStats True if eBPF is in use.
*/
public static void apply464xlatAdjustments(NetworkStats baseTraffic,
- NetworkStats stackedTraffic, Map<String, String> stackedIfaces) {
+ NetworkStats stackedTraffic, Map<String, String> stackedIfaces, boolean useBpfStats) {
// Total 464xlat traffic to subtract from uid 0 on all base interfaces.
// stackedIfaces may grow afterwards, but NetworkStats will just be resized automatically.
final NetworkStats adjustments = new NetworkStats(0, stackedIfaces.size());
@@ -837,15 +842,20 @@
continue;
}
// Subtract any 464lat traffic seen for the root UID on the current base interface.
+ // However, for eBPF, the per uid stats is collected by different hook, the rx packets
+ // on base interface will not be counted. Thus, the adjustment on root uid is only
+ // needed in tx direction.
adjust.iface = baseIface;
- adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA);
+ if (!useBpfStats) {
+ adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA);
+ adjust.rxPackets = -entry.rxPackets;
+ }
adjust.txBytes = -(entry.txBytes + entry.txPackets * IPV4V6_HEADER_DELTA);
- adjust.rxPackets = -entry.rxPackets;
adjust.txPackets = -entry.txPackets;
adjustments.combineValues(adjust);
- // For 464xlat traffic, xt_qtaguid only counts the bytes of the native IPv4 packet sent
- // on the stacked interface with prefix "v4-" and drops the IPv6 header size after
+ // For 464xlat traffic, per uid stats only counts the bytes of the native IPv4 packet
+ // sent on the stacked interface with prefix "v4-" and drops the IPv6 header size after
// unwrapping. To account correctly for on-the-wire traffic, add the 20 additional bytes
// difference for all packets (http://b/12249687, http:/b/33681750).
entry.rxBytes += entry.rxPackets * IPV4V6_HEADER_DELTA;
@@ -864,8 +874,8 @@
* base and stacked traffic.
* @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both.
*/
- public void apply464xlatAdjustments(Map<String, String> stackedIfaces) {
- apply464xlatAdjustments(this, this, stackedIfaces);
+ public void apply464xlatAdjustments(Map<String, String> stackedIfaces, boolean useBpfStats) {
+ apply464xlatAdjustments(this, this, stackedIfaces, useBpfStats);
}
/**
diff --git a/core/java/com/android/internal/net/NetworkStatsFactory.java b/core/java/com/android/internal/net/NetworkStatsFactory.java
index d1c2799..0a7cff6 100644
--- a/core/java/com/android/internal/net/NetworkStatsFactory.java
+++ b/core/java/com/android/internal/net/NetworkStatsFactory.java
@@ -113,11 +113,12 @@
/**
* Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}.
- * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map)
+ * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean)
*/
public static void apply464xlatAdjustments(NetworkStats baseTraffic,
- NetworkStats stackedTraffic) {
- NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces);
+ NetworkStats stackedTraffic, boolean useBpfStats) {
+ NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces,
+ useBpfStats);
}
@VisibleForTesting
@@ -263,7 +264,7 @@
// No locking here: apply464xlatAdjustments behaves fine with an add-only ConcurrentHashMap.
// TODO: remove this and only apply adjustments in NetworkStatsService.
- stats.apply464xlatAdjustments(sStackedIfaces);
+ stats.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats);
return stats;
}
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 3f03169..383e1e0 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -1620,7 +1620,8 @@
// 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);
+ NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot,
+ mUseBpfTrafficStats);
uidSnapshot.combineAllValues(tetherSnapshot);
final TelephonyManager telephonyManager = (TelephonyManager) mContext.getSystemService(
@@ -1630,7 +1631,8 @@
final NetworkStats vtStats = telephonyManager.getVtDataUsage(STATS_PER_UID);
if (vtStats != null) {
vtStats.filter(UID_ALL, ifaces, TAG_ALL);
- NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats);
+ NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats,
+ mUseBpfTrafficStats);
uidSnapshot.combineAllValues(vtStats);
}
diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java
index 8f18d07..d6dbf5a 100644
--- a/tests/net/java/android/net/NetworkStatsTest.java
+++ b/tests/net/java/android/net/NetworkStatsTest.java
@@ -39,6 +39,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import android.net.NetworkStats.Entry;
import android.os.Process;
import android.support.test.runner.AndroidJUnit4;
import android.support.test.filters.SmallTest;
@@ -785,7 +786,38 @@
ArrayMap<String, String> stackedIface = new ArrayMap<>();
stackedIface.put(v4Iface, baseIface);
- NetworkStats.Entry otherEntry = new NetworkStats.Entry(
+ // Ipv4 traffic sent/received by an app on stacked interface.
+ final NetworkStats.Entry appEntry = new NetworkStats.Entry(
+ v4Iface, appUid, SET_DEFAULT, TAG_NONE,
+ 30501490 /* rxBytes */,
+ 22401 /* rxPackets */,
+ 876235 /* txBytes */,
+ 13805 /* txPackets */,
+ 0 /* operations */);
+
+ // Traffic measured for the root uid on the base interface if eBPF is in use.
+ // Incorrectly includes appEntry's bytes and packets, plus IPv4-IPv6 translation
+ // overhead (20 bytes per packet), only for TX traffic.
+ final NetworkStats.Entry ebpfRootUidEntry = new NetworkStats.Entry(
+ baseIface, rootUid, SET_DEFAULT, TAG_NONE,
+ 163577 /* rxBytes */,
+ 187 /* rxPackets */,
+ 1169942 /* txBytes */,
+ 13902 /* txPackets */,
+ 0 /* operations */);
+
+ // Traffic measured for the root uid on the base interface if xt_qtaguid is in use.
+ // Incorrectly includes appEntry's bytes and packets, plus IPv4-IPv6 translation
+ // overhead (20 bytes per packet), in both directions.
+ final NetworkStats.Entry xtRootUidEntry = new NetworkStats.Entry(
+ baseIface, rootUid, SET_DEFAULT, TAG_NONE,
+ 31113087 /* rxBytes */,
+ 22588 /* rxPackets */,
+ 1169942 /* txBytes */,
+ 13902 /* txPackets */,
+ 0 /* operations */);
+
+ final NetworkStats.Entry otherEntry = new NetworkStats.Entry(
otherIface, appUid, SET_DEFAULT, TAG_NONE,
2600 /* rxBytes */,
2 /* rxPackets */,
@@ -793,39 +825,41 @@
3 /* txPackets */,
0 /* operations */);
- NetworkStats stats = new NetworkStats(TEST_START, 3)
- .addValues(v4Iface, appUid, SET_DEFAULT, TAG_NONE,
- 30501490 /* rxBytes */,
- 22401 /* rxPackets */,
- 876235 /* txBytes */,
- 13805 /* txPackets */,
- 0 /* operations */)
- .addValues(baseIface, rootUid, SET_DEFAULT, TAG_NONE,
- 31113087,
- 22588,
- 1169942,
- 13902,
- 0)
+ final NetworkStats statsXt = new NetworkStats(TEST_START, 3)
+ .addValues(appEntry)
+ .addValues(xtRootUidEntry)
.addValues(otherEntry);
- stats.apply464xlatAdjustments(stackedIface);
+ final NetworkStats statsEbpf = new NetworkStats(TEST_START, 3)
+ .addValues(appEntry)
+ .addValues(ebpfRootUidEntry)
+ .addValues(otherEntry);
- assertEquals(3, stats.size());
- assertValues(stats, 0, v4Iface, appUid, SET_DEFAULT, TAG_NONE,
- METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO,
+ statsXt.apply464xlatAdjustments(stackedIface, false);
+ statsEbpf.apply464xlatAdjustments(stackedIface, true);
+
+ assertEquals(3, statsXt.size());
+ assertEquals(3, statsEbpf.size());
+ final NetworkStats.Entry expectedAppUid = new NetworkStats.Entry(
+ v4Iface, appUid, SET_DEFAULT, TAG_NONE,
30949510,
22401,
1152335,
13805,
0);
- assertValues(stats, 1, baseIface, 0, SET_DEFAULT, TAG_NONE,
- METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO,
+ final NetworkStats.Entry expectedRootUid = new NetworkStats.Entry(
+ baseIface, 0, SET_DEFAULT, TAG_NONE,
163577,
187,
17607,
97,
0);
- assertEquals(otherEntry, stats.getValues(2, null));
+ assertEquals(expectedAppUid, statsXt.getValues(0, null));
+ assertEquals(expectedRootUid, statsXt.getValues(1, null));
+ assertEquals(otherEntry, statsXt.getValues(2, null));
+ assertEquals(expectedAppUid, statsEbpf.getValues(0, null));
+ assertEquals(expectedRootUid, statsEbpf.getValues(1, null));
+ assertEquals(otherEntry, statsEbpf.getValues(2, null));
}
@Test
@@ -850,7 +884,7 @@
.addValues(secondEntry);
// Empty map: no adjustment
- stats.apply464xlatAdjustments(new ArrayMap<>());
+ stats.apply464xlatAdjustments(new ArrayMap<>(), false);
assertEquals(2, stats.size());
assertEquals(firstEntry, stats.getValues(0, null));