From 5ea66a0a55535a67c14cf2beb106b45c4c5b091d Mon Sep 17 00:00:00 2001 From: junyulai Date: Fri, 19 Oct 2018 21:14:30 +0800 Subject: [PATCH] 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 Merged-In: Ic9a84f5446eddc943c255d5f3b89dad171f53cac (cherry picked from commit c33ac0d43b594f6154accf03ae7e3fd34dedc79d) (cherry picked from commit 3da40c4828f7311e58ccc0a52d0a1f2122beaed8) --- core/java/android/net/NetworkStats.java | 24 +++++++++++++------ .../internal/net/NetworkStatsFactory.java | 9 +++---- .../server/net/NetworkStatsService.java | 6 +++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index edf9bc1c6b..4bcc245368 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -776,14 +776,19 @@ public class NetworkStats implements Parcelable { * 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. + * *

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 stackedIfaces) { + NetworkStats stackedTraffic, Map 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()); @@ -802,15 +807,20 @@ public class NetworkStats implements Parcelable { 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; @@ -829,8 +839,8 @@ public class NetworkStats implements Parcelable { * base and stacked traffic. * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both. */ - public void apply464xlatAdjustments(Map stackedIfaces) { - apply464xlatAdjustments(this, this, stackedIfaces); + public void apply464xlatAdjustments(Map 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 c4d08c7dc5..b3cf389c1b 100644 --- a/core/java/com/android/internal/net/NetworkStatsFactory.java +++ b/core/java/com/android/internal/net/NetworkStatsFactory.java @@ -113,11 +113,12 @@ public class NetworkStatsFactory { /** * 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 @@ public class NetworkStatsFactory { // 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 9bf3874088..17ab29d747 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -1622,7 +1622,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // 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( @@ -1632,7 +1633,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { 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); }