Fix negative uid stats caused by 464xlat adjust when eBPF is on.
When using xt_qtaguid to count per uid stats,
NetworkStatsService needs to adjust the 464xlat traffic since
iptables module would double count for ipv4 and ipv6 packet.
But for eBPF, the per uid stats is collected in a different
hook, so the adjustment on root uid would only be needed in tx
direction.
Bug: 112226716
Test: 1. Make ipv4 traffic in ipv6-only network and check data
usage.
2. Make ipv4 traffic in a client which connect to
ipv6-only hotspot.
3. runtest frameworks-net
4. cts-tradefed run cts -m CtsNetTestCases -t \
android.net.cts.TrafficStatsTest
5. cts-tradefed run cts -m CtsUsageStatsTestCases
Change-Id: Ic9a84f5446eddc943c255d5f3b89dad171f53cac
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));