From d1dd2f9cf3f51c2a0b59e8750b99b51fc44ff57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Tue, 24 Dec 2019 02:50:35 -0800 Subject: [PATCH 1/7] NetworkStats - no need for xt_qtaguid clat bw fixups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is due to the previous addition of the ip6tables raw PREROUTING drop rules for incoming ipv6 clat traffic pre-translation to ipv4. Since we no longer double account, we no longer need these fixups. Test: atest bpf_module_test clatd_test libbpf_android_test libnetdbpf_test netd_integration_test netd_unit_test netdutils_test Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: Ia171b7797cdc99367064d0649bf1293c71579941 Merged-In: Ia171b7797cdc99367064d0649bf1293c71579941 --- core/java/android/net/NetworkStats.java | 64 ++++++++++--------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index b7fb280efdc41..3141320a167d1 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -1047,22 +1047,22 @@ public final class NetworkStats implements Parcelable { } /** - * Calculate and apply adjustments to captured statistics for 464xlat traffic counted twice. + * Calculate and apply adjustments to captured statistics for 464xlat traffic. * - *

This mutates both base and stacked traffic stats, to account respectively for - * double-counted traffic and IPv4/IPv6 header size difference. + *

This mutates stacked traffic stats, to account for IPv4/IPv6 header size difference. * - *

For 464xlat traffic, xt_qtaguid sees every IPv4 packet twice, once as a native IPv4 - * packet on the stacked interface, and once as translated to an IPv6 packet on the - * base interface. For correct stats accounting on the base interface, if using xt_qtaguid, - * every rx 464xlat packet needs to be subtracted from the root UID on the base interface - * (http://b/12249687, http:/b/33681750), and every tx 464xlat packet which was counted onto - * clat uid should be ignored. + *

UID stats, which are only accounted on the stacked interface, need to be increased + * by 20 bytes/packet to account for translation overhead. * - * 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 not needed. However, the - * tx traffic counted in the same way xt_qtaguid does, so the traffic on clat uid still - * needs to be ignored. + *

The potential additional overhead of 8 bytes/packet for ip fragments is ignored. + * + *

Interface stats need to sum traffic on both stacked and base interface because: + * - eBPF offloaded packets appear only on the stacked interface + * - Non-offloaded ingress packets appear only on the stacked interface + * (due to iptables raw PREROUTING drop rules) + * - Non-offloaded egress packets appear only on the stacked interface + * (due to ignoring traffic from clat daemon by uid match) + * (and of course the 20 bytes/packet overhead needs to be applied to stacked interface stats) * *

This method will behave fine if {@code stackedIfaces} is an non-synchronized but add-only * {@code ConcurrentHashMap} @@ -1074,46 +1074,34 @@ public final class NetworkStats implements Parcelable { */ public static void apply464xlatAdjustments(NetworkStats baseTraffic, 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()); - // For recycling Entry entry = null; - Entry adjust = new NetworkStats.Entry(IFACE_ALL, 0, 0, 0, 0, 0, 0, 0L, 0L, 0L, 0L, 0L); - for (int i = 0; i < stackedTraffic.size; i++) { entry = stackedTraffic.getValues(i, entry); - if (entry.iface == null || !entry.iface.startsWith(CLATD_INTERFACE_PREFIX)) { - continue; - } - final String baseIface = stackedIfaces.get(entry.iface); - if (baseIface == null) { - continue; - } - // Subtract xt_qtaguid 464lat rx traffic seen for the root UID on the current base - // interface. As for eBPF, the per uid stats is collected by different hook, the rx - // packets on base interface will not be counted. - adjust.iface = baseIface; - if (!useBpfStats) { - adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA); - adjust.rxPackets = -entry.rxPackets; - } - adjustments.combineValues(adjust); + if (entry == null) continue; + if (entry.iface == null) continue; + if (!entry.iface.startsWith(CLATD_INTERFACE_PREFIX)) continue; // 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). + // + // Note: this doesn't account for LRO/GRO/GSO/TSO (ie. >mtu) traffic correctly, nor + // does it correctly account for the 8 extra bytes in the IPv6 fragmentation header. + // + // While the ebpf code path does try to simulate proper post segmentation packet + // counts, we have nothing of the sort of xt_qtaguid stats. entry.rxBytes += entry.rxPackets * IPV4V6_HEADER_DELTA; entry.txBytes += entry.txPackets * IPV4V6_HEADER_DELTA; stackedTraffic.setValues(i, entry); } - // Traffic on clat uid is v6 tx traffic that is already counted with app uid on the stacked - // v4 interface, so it needs to be removed to avoid double-counting. + // Theoretically there should be no traffic accounted to the clat daemon's uid: + // see ebpf program 'netd.c's early returns + // and iptables '-m owner --uid-owner clat -j RETURN' rules prior to accounting + // TODO: remove this - should definitely be safe once ebpf only. baseTraffic.removeUids(new int[] {CLAT_UID}); - baseTraffic.combineAllValues(adjustments); } /** From 6affb9bb0195fb58f8aa86e23263056fe366d87a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 4 Jan 2020 13:20:53 -0800 Subject: [PATCH 2/7] fix android.net.NetworkStatsTest#testApply464xlatAdjustments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: atest NetworkStatsTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: Ib12ee88295eb502f6da13d212b4fd6298dea380f Merged-In: Ib12ee88295eb502f6da13d212b4fd6298dea380f --- .../java/android/net/NetworkStatsTest.java | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java index 98f705f45e980..f28b1864c11c1 100644 --- a/tests/net/java/android/net/NetworkStatsTest.java +++ b/tests/net/java/android/net/NetworkStatsTest.java @@ -909,8 +909,8 @@ public class NetworkStatsTest { 13805 /* txPackets */, 0 /* operations */); - // Traffic measured for the root uid on the base interface if eBPF is in use. - final NetworkStats.Entry ebpfRootUidEntry = new NetworkStats.Entry( + // Traffic measured for the root uid on the base interface. + final NetworkStats.Entry rootUidEntry = new NetworkStats.Entry( baseIface, rootUid, SET_DEFAULT, TAG_NONE, 163577 /* rxBytes */, 187 /* rxPackets */, @@ -918,17 +918,6 @@ public class NetworkStatsTest { 97 /* 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 rx direction. - final NetworkStats.Entry xtRootUidEntry = new NetworkStats.Entry( - baseIface, rootUid, SET_DEFAULT, TAG_NONE, - 31113087 /* rxBytes */, - 22588 /* rxPackets */, - 17607 /* txBytes */, - 97 /* txPackets */, - 0 /* operations */); - final NetworkStats.Entry otherEntry = new NetworkStats.Entry( otherIface, appUid, SET_DEFAULT, TAG_NONE, 2600 /* rxBytes */, @@ -939,12 +928,12 @@ public class NetworkStatsTest { final NetworkStats statsXt = new NetworkStats(TEST_START, 3) .insertEntry(appEntry) - .insertEntry(xtRootUidEntry) + .insertEntry(rootUidEntry) .insertEntry(otherEntry); final NetworkStats statsEbpf = new NetworkStats(TEST_START, 3) .insertEntry(appEntry) - .insertEntry(ebpfRootUidEntry) + .insertEntry(rootUidEntry) .insertEntry(otherEntry); statsXt.apply464xlatAdjustments(stackedIface, false); From 68d998de26ba5d77d85e29fccb6edd81d141946e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 4 Jan 2020 12:51:49 -0800 Subject: [PATCH 3/7] fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccountingSimple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No clat app uid 0 rx stats on base iface due to ip6tables raw prerouting drop Test: atest NetworkStatsFactoryTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: Ifebd946e06b5fa2eeb7ff4fc0b1c423097021692 Merged-In: Ifebd946e06b5fa2eeb7ff4fc0b1c423097021692 --- tests/net/res/raw/xt_qtaguid_with_clat_simple | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/net/res/raw/xt_qtaguid_with_clat_simple b/tests/net/res/raw/xt_qtaguid_with_clat_simple index b37fae6d2a3d2..2c9b9f768dd72 100644 --- a/tests/net/res/raw/xt_qtaguid_with_clat_simple +++ b/tests/net/res/raw/xt_qtaguid_with_clat_simple @@ -1,5 +1,5 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packets rx_tcp_bytes rx_tcp_packets rx_udp_bytes rx_udp_packets rx_other_bytes rx_other_packets tx_tcp_bytes tx_tcp_packets tx_udp_bytes tx_udp_packets tx_other_bytes tx_other_packets 2 v4-wlan0 0x0 10060 0 42600 213 4100 41 42600 213 0 0 0 0 4100 41 0 0 0 0 3 v4-wlan0 0x0 10060 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -4 wlan0 0x0 0 0 46860 213 0 0 46860 213 0 0 0 0 0 0 0 0 0 0 +4 wlan0 0x0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 5 wlan0 0x0 1029 0 0 0 4920 41 0 0 0 0 0 0 4920 41 0 0 0 0 From c9e66053203c3b4471a04e405cbb90ab77af2b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 4 Jan 2020 12:59:06 -0800 Subject: [PATCH 4/7] fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccounting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No clat app uid 0 rx stats on base iface due to ip6tables raw prerouting drop Test: atest NetworkStatsFactoryTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: I4393afcbe8e1bb886ecf4f5c1573f6ac8a0e29f5 Merged-In: I4393afcbe8e1bb886ecf4f5c1573f6ac8a0e29f5 --- .../java/com/android/server/net/NetworkStatsFactoryTest.java | 2 +- tests/net/res/raw/xt_qtaguid_with_clat | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java index 4473492d7972f..fd51c4ae3f576 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java @@ -446,7 +446,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest { assertStatsEntry(stats, "v4-wlan0", 1000, SET_DEFAULT, 0x0, 30812L, 2310L); assertStatsEntry(stats, "v4-wlan0", 10102, SET_DEFAULT, 0x0, 10022L, 3330L); assertStatsEntry(stats, "v4-wlan0", 10060, SET_DEFAULT, 0x0, 9532772L, 254112L); - assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, 15229L, 0L); + assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, 0L, 0L); assertStatsEntry(stats, "wlan0", 1000, SET_DEFAULT, 0x0, 6126L, 2013L); assertStatsEntry(stats, "wlan0", 10013, SET_DEFAULT, 0x0, 0L, 144L); assertStatsEntry(stats, "wlan0", 10018, SET_DEFAULT, 0x0, 5980263L, 167667L); diff --git a/tests/net/res/raw/xt_qtaguid_with_clat b/tests/net/res/raw/xt_qtaguid_with_clat index 6cd7499545be9..8967d4f1ac4a9 100644 --- a/tests/net/res/raw/xt_qtaguid_with_clat +++ b/tests/net/res/raw/xt_qtaguid_with_clat @@ -7,7 +7,7 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe 7 v4-wlan0 0x0 10060 1 1448660 1041 31192 753 1448660 1041 0 0 0 0 31192 753 0 0 0 0 8 v4-wlan0 0x0 10102 0 9702 16 2870 23 9702 16 0 0 0 0 2870 23 0 0 0 0 9 v4-wlan0 0x0 10102 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -10 wlan0 0x0 0 0 11058671 7892 0 0 11043898 7811 13117 61 1656 20 0 0 0 0 0 0 +10 wlan0 0x0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 11 wlan0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 12 wlan0 0x0 1000 0 6126 13 2013 16 5934 11 192 2 0 0 1821 14 192 2 0 0 13 wlan0 0x0 1000 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 From e07aa4a466c1ca82dc015905f9291efff705f01b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 4 Jan 2020 13:07:51 -0800 Subject: [PATCH 5/7] fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccounting100MBDownload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No clat app uid 0 rx stats on base iface due to ip6tables raw prerouting drop (so just copy over the rx stats from the before file) Test: atest NetworkStatsTest NetworkStatsFactoryTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: Ie73ba0586dafde67bc2726db26e07f7268c18be3 Merged-In: Ie73ba0586dafde67bc2726db26e07f7268c18be3 --- .../com/android/server/net/NetworkStatsFactoryTest.java | 8 +++----- .../net/res/raw/xt_qtaguid_with_clat_100mb_download_after | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java index fd51c4ae3f576..e4996d981fac0 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java @@ -468,9 +468,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest { long appRxBytesAfter = 439237478L; assertEquals("App traffic should be ~100MB", 110553449, appRxBytesAfter - appRxBytesBefore); - long rootRxBytesBefore = 1394011L; - long rootRxBytesAfter = 1398634L; - assertEquals("UID 0 traffic should be ~0", 4623, rootRxBytesAfter - rootRxBytesBefore); + long rootRxBytes = 330187296L; mFactory.noteStackedIface("v4-wlan0", "wlan0"); NetworkStats stats; @@ -478,12 +476,12 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest { // Stats snapshot before the download stats = parseDetailedStats(R.raw.xt_qtaguid_with_clat_100mb_download_before); assertStatsEntry(stats, "v4-wlan0", 10106, SET_FOREGROUND, 0x0, appRxBytesBefore, 5199872L); - assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytesBefore, 0L); + assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytes, 0L); // Stats snapshot after the download stats = parseDetailedStats(R.raw.xt_qtaguid_with_clat_100mb_download_after); assertStatsEntry(stats, "v4-wlan0", 10106, SET_FOREGROUND, 0x0, appRxBytesAfter, 7867488L); - assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytesAfter, 0L); + assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytes, 0L); } /** diff --git a/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after b/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after index 9f86153a33cfb..12d98ca29f571 100644 --- a/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after +++ b/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after @@ -9,7 +9,7 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe 9 v4-wlan0 0x0 10057 1 728 7 392 7 0 0 728 7 0 0 0 0 392 7 0 0 10 v4-wlan0 0x0 10106 0 2232 18 2232 18 0 0 2232 18 0 0 0 0 2232 18 0 0 11 v4-wlan0 0x0 10106 1 432952718 314238 5442288 121260 432950238 314218 2480 20 0 0 5433900 121029 8388 231 0 0 -12 wlan0 0x0 0 0 440746376 329772 0 0 439660007 315369 232001 1276 854368 13127 0 0 0 0 0 0 +12 wlan0 0x0 0 0 330187296 250652 0 0 329106990 236273 226202 1255 854104 13124 0 0 0 0 0 0 13 wlan0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 14 wlan0 0x0 1000 0 77113 272 56151 575 77113 272 0 0 0 0 19191 190 36960 385 0 0 15 wlan0 0x0 1000 1 20227 80 8356 72 18539 74 1688 6 0 0 7562 66 794 6 0 0 From d09cef39c5511bfe20ec6615423bf47cbfc1a5de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 28 May 2020 01:06:28 -0700 Subject: [PATCH 6/7] NetworkStats: apply464xlatAdjustments - remove useBpfStats parameter. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: atest NetworkStatsTest NetworkStatsFactoryTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: I0121a4ac7ee824adc5930bab786d550b2f00b05b Merged-In: I0121a4ac7ee824adc5930bab786d550b2f00b05b --- core/java/android/net/NetworkStats.java | 7 +++--- .../server/net/NetworkStatsFactory.java | 10 ++++---- .../server/net/NetworkStatsService.java | 5 ++-- .../java/android/net/NetworkStatsTest.java | 24 ++++++------------- 4 files changed, 16 insertions(+), 30 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index 3141320a167d1..ea0889276c5c9 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -1069,11 +1069,10 @@ public final class NetworkStats implements Parcelable { * @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. * @hide */ public static void apply464xlatAdjustments(NetworkStats baseTraffic, - NetworkStats stackedTraffic, Map stackedIfaces, boolean useBpfStats) { + NetworkStats stackedTraffic, Map stackedIfaces) { // For recycling Entry entry = null; for (int i = 0; i < stackedTraffic.size; i++) { @@ -1113,8 +1112,8 @@ public final class NetworkStats implements Parcelable { * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both. * @hide */ - public void apply464xlatAdjustments(Map stackedIfaces, boolean useBpfStats) { - apply464xlatAdjustments(this, this, stackedIfaces, useBpfStats); + public void apply464xlatAdjustments(Map stackedIfaces) { + apply464xlatAdjustments(this, this, stackedIfaces); } /** diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java index 3dac106418c52..86ad0b3086763 100644 --- a/services/core/java/com/android/server/net/NetworkStatsFactory.java +++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java @@ -152,12 +152,10 @@ public class NetworkStatsFactory { /** * Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}. - * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean) + * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map) */ - public void apply464xlatAdjustments(NetworkStats baseTraffic, - NetworkStats stackedTraffic, boolean useBpfStats) { - NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces, - useBpfStats); + public void apply464xlatAdjustments(NetworkStats baseTraffic, NetworkStats stackedTraffic) { + NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces); } public NetworkStatsFactory() { @@ -380,7 +378,7 @@ public class NetworkStatsFactory { // network, the overhead is their fault. // No locking here: apply464xlatAdjustments behaves fine with an add-only // ConcurrentHashMap. - delta.apply464xlatAdjustments(mStackedIfaces, mUseBpfStats); + delta.apply464xlatAdjustments(mStackedIfaces); // Migrate data usage over a VPN to the TUN network. for (VpnInfo info : vpnArray) { diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index cf5a412321232..ba9f486092f7f 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -1882,14 +1882,13 @@ 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); - mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot, - mUseBpfTrafficStats); + mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot); uidSnapshot.combineAllValues(tetherSnapshot); // get a stale copy of uid stats snapshot provided by providers. final NetworkStats providerStats = getNetworkStatsFromProviders(STATS_PER_UID); providerStats.filter(UID_ALL, ifaces, TAG_ALL); - mStatsFactory.apply464xlatAdjustments(uidSnapshot, providerStats, mUseBpfTrafficStats); + mStatsFactory.apply464xlatAdjustments(uidSnapshot, providerStats); uidSnapshot.combineAllValues(providerStats); uidSnapshot.combineAllValues(mUidOperations); diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java index f28b1864c11c1..735fa7cf37514 100644 --- a/tests/net/java/android/net/NetworkStatsTest.java +++ b/tests/net/java/android/net/NetworkStatsTest.java @@ -926,21 +926,14 @@ public class NetworkStatsTest { 3 /* txPackets */, 0 /* operations */); - final NetworkStats statsXt = new NetworkStats(TEST_START, 3) + final NetworkStats stats = new NetworkStats(TEST_START, 3) .insertEntry(appEntry) .insertEntry(rootUidEntry) .insertEntry(otherEntry); - final NetworkStats statsEbpf = new NetworkStats(TEST_START, 3) - .insertEntry(appEntry) - .insertEntry(rootUidEntry) - .insertEntry(otherEntry); + stats.apply464xlatAdjustments(stackedIface); - statsXt.apply464xlatAdjustments(stackedIface, false); - statsEbpf.apply464xlatAdjustments(stackedIface, true); - - assertEquals(3, statsXt.size()); - assertEquals(3, statsEbpf.size()); + assertEquals(3, stats.size()); final NetworkStats.Entry expectedAppUid = new NetworkStats.Entry( v4Iface, appUid, SET_DEFAULT, TAG_NONE, 30949510, @@ -955,12 +948,9 @@ public class NetworkStatsTest { 17607, 97, 0); - 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)); + assertEquals(expectedAppUid, stats.getValues(0, null)); + assertEquals(expectedRootUid, stats.getValues(1, null)); + assertEquals(otherEntry, stats.getValues(2, null)); } @Test @@ -985,7 +975,7 @@ public class NetworkStatsTest { .insertEntry(secondEntry); // Empty map: no adjustment - stats.apply464xlatAdjustments(new ArrayMap<>(), false); + stats.apply464xlatAdjustments(new ArrayMap<>()); assertEquals(2, stats.size()); assertEquals(firstEntry, stats.getValues(0, null)); From b2603a31939f737723bdf93fd4c3bb4df65d5a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 28 May 2020 01:33:52 -0700 Subject: [PATCH 7/7] NetworkStats: apply464xlatAdjustments - don't remove CLAT_UID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should no longer be needed after all the cleanups. Test: atest NetworkStatsTest NetworkStatsFactoryTest Bug: 150738490 Signed-off-by: Maciej Żenczykowski Change-Id: I289d935f84b616ed857ef4c5a7427d57c282d00c Merged-In: I289d935f84b616ed857ef4c5a7427d57c282d00c --- core/java/android/net/NetworkStats.java | 8 -------- tests/net/res/raw/xt_qtaguid_with_clat | 2 -- tests/net/res/raw/xt_qtaguid_with_clat_simple | 1 - 3 files changed, 11 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index ea0889276c5c9..34e48eb44f327 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -16,8 +16,6 @@ package android.net; -import static android.os.Process.CLAT_UID; - import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -1095,12 +1093,6 @@ public final class NetworkStats implements Parcelable { entry.txBytes += entry.txPackets * IPV4V6_HEADER_DELTA; stackedTraffic.setValues(i, entry); } - - // Theoretically there should be no traffic accounted to the clat daemon's uid: - // see ebpf program 'netd.c's early returns - // and iptables '-m owner --uid-owner clat -j RETURN' rules prior to accounting - // TODO: remove this - should definitely be safe once ebpf only. - baseTraffic.removeUids(new int[] {CLAT_UID}); } /** diff --git a/tests/net/res/raw/xt_qtaguid_with_clat b/tests/net/res/raw/xt_qtaguid_with_clat index 8967d4f1ac4a9..f04b32f08332d 100644 --- a/tests/net/res/raw/xt_qtaguid_with_clat +++ b/tests/net/res/raw/xt_qtaguid_with_clat @@ -41,5 +41,3 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe 41 dummy0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 42 lo 0x0 0 0 1288 16 1288 16 0 0 532 8 756 8 0 0 532 8 756 8 43 lo 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -44 wlan0 0x0 1029 0 0 0 312046 5113 0 0 0 0 0 0 306544 5046 3230 38 2272 29 -45 wlan0 0x0 1029 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \ No newline at end of file diff --git a/tests/net/res/raw/xt_qtaguid_with_clat_simple b/tests/net/res/raw/xt_qtaguid_with_clat_simple index 2c9b9f768dd72..a1d6d411bad81 100644 --- a/tests/net/res/raw/xt_qtaguid_with_clat_simple +++ b/tests/net/res/raw/xt_qtaguid_with_clat_simple @@ -2,4 +2,3 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe 2 v4-wlan0 0x0 10060 0 42600 213 4100 41 42600 213 0 0 0 0 4100 41 0 0 0 0 3 v4-wlan0 0x0 10060 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 4 wlan0 0x0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -5 wlan0 0x0 1029 0 0 0 4920 41 0 0 0 0 0 0 4920 41 0 0 0 0